Hi Arthur!

On 2022-03-02T13:05:30+0100, Arthur Cohen <arthur.co...@embecosm.com> wrote:
> On 3/2/22 11:05, Thomas Schwinge wrote:
>> On 2022-03-02T10:44:38+0100, I wrote:
>>> On 2022-03-02T09:03:48+0000, Philip Herron <philip.her...@embecosm.com> 
>>> wrote:
>>>> Yet again the build bots are out doing github automation :D. Would you
>>>> be able to give Arthur access to the failing buildbot to test his fix?
>>>
>>> I suggest as a first step to figure out why your local build aren't
>>> running into this issue.  What does running 'build-gcc/gcc/xgcc -v'
>>> should for you, and 'grep -i checking build-gcc/gcc/auto-host.h'?
>>>
>>>> We think we know what the issue is. We changed the lexer so we could
>>>> give it a buffer instead of a file for macro expansion, but the string
>>>> is going out of scope when we set it up.
>>>
>>> Ah!  So, "standard C/C++ undefined behavior, memory corruption"...  ;-)
>>
>> Indeed.  Building GCC with '--enable-valgrind-annotations' and running
>> the GCC self-test through '-wrapper valgrind,--leak-check=full', we see:
>>
>>      $ [...]/build-gcc/./gcc/xgcc -B[...]/build-gcc/./gcc/ -xrs -nostdinc 
>> /dev/null -S -o /dev/null 
>> -fself-test=[...]/source-gcc/gcc/testsuite/selftests -wrapper 
>> valgrind,--leak-check=full
>>      ==3228208== Memcheck, a memory error detector
>>      ==3228208== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et 
>> al.
>>      ==3228208== Using Valgrind-3.18.1 and LibVEX; rerun with -h for 
>> copyright info
>>      ==3228208== Command: [...]/build-gcc/./gcc/rust1 /dev/null -quiet 
>> -dumpbase null -mtune=generic -march=x86-64 
>> -fself-test=[...]/source-gcc/gcc/testsuite/selftests -o /dev/null 
>> -L[...]/build-gcc/./gcc -L/lib/x86_64-linux-gnu -L/lib/../lib64 
>> -L/usr/lib/x86_64-linux-gnu
>>      ==3228208==
>>      rust1: error: unexpected character ‘1’
>>      rust1: error: unexpected character ‘0’
>>      rust1: error: unexpected character ‘0’
>>      rust1: error: unexpected character ‘0’
>>      rust1: error: unexpected character ‘1’
>>      rust1: error: unexpected character ‘0’
>>      rust1: error: unexpected character ‘0’
>>      rust1: error: unexpected character ‘0’
>>      ==3228208== Conditional jump or move depends on uninitialised value(s)
>>      ==3228208==    at 0x983D5E: Rust::Lexer::build_token() (rust-lex.cc:365)
>>      ==3228208==    by 0x987DB4: 
>> Rust::buffered_queue<std::shared_ptr<Rust::Token>, 
>> Rust::Lexer::TokenSource>::peek(int) (rust-lex.h:233)
>>      ==3228208==    by 0x988A46: 
>> Rust::parse_cfg_option(std::__cxx11::basic_string<char, 
>> std::char_traits<char>, std::allocator<char> >&, 
>> std::__cxx11::basic_string<char, std::char_traits<char>, 
>> std::allocator<char> >&, std::__cxx11::basic_string<char, 
>> std::char_traits<char>, std::allocator<char> >&) (rust-lex.h:166)
>>      ==3228208==    by 0x988D83: selftest::rust_cfg_parser_test() 
>> (rust-cfg-parser.cc:67)
>>      ==3228208==    by 0x96C5EE: selftest::run_rust_tests() 
>> (rust-lang.cc:457)
>>      ==3228208==    by 0x2527223: selftest::run_tests() 
>> (selftest-run-tests.cc:119)
>>      ==3228208==    by 0x11C2C29: toplev::run_self_tests() (toplev.cc:2217)
>>      ==3228208==    by 0x96991B: toplev::main(int, char**) (toplev.cc:2317)
>>      ==3228208==    by 0x96BFD6: main (main.cc:39)
>>      [...]
>>
>> Before commit 6cf9f8c99c5813a23d7cec473fedf00683f409e4 "Merge #983", that
>> was "clean" (just some lost memory etc.).
>
> Thanks a lot for this. I thought it was some memory corruption,
> considering the lexer was seeing garbage characters. I'm a bit
> disappointed my computer and github's CI didn't have the behaviour, but
> oh well.

But have you been able to reproduce the issue with Valgrind, per the
instructiosn I've given?  (That's generally useful in GCC development: to
know about '--enable-valgrind-annotations', how to use '-wrapper' for
Valgrind but also GDB, etc.)

> I'd like to say that this is not my fault

Heh, no worries!  ;-)

> and that there really should
> be a wrapper around FILE* operations in the standard C++ library, or
> that this would have been avoided had we been writing Rust, but sadly... :)
>
> I think the patch to test is pretty easy if any of you guys want to test
> it. I'll also send a public key to Mark, thank you for the offer:
>
> ---
>   gcc/rust/lex/rust-lex.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/rust/lex/rust-lex.h b/gcc/rust/lex/rust-lex.h
> index c50f63208de..b0d7494f063 100644
> --- a/gcc/rust/lex/rust-lex.h
> +++ b/gcc/rust/lex/rust-lex.h
> @@ -144,7 +144,7 @@ public:
>     /**
>      * Lex the contents of a string instead of a file
>      */
> -  static Lexer lex_string (std::string input)
> +  static Lexer lex_string (std::string &input)
>     {
>       // We can perform this ugly cast to a non-const char* since we're only
>       // *reading* the string. This would not be valid if we were doing any

That is, revert commit f7ff6020f8c68e4fb54c17c4460aa7f8a31f85bd
"lexer: Improve safety by taking ownership of the tokenized string".  ;-)

> This forces the string to simply be a reference and the caller to make
> sure that it outlives the lexer.
> If this passes the valgrind tests, I'll open up a PR with a comment
> explaining the behavior and how we should think about cleaning
> it up.

I don't have time right now to think throught the C++ aspects, but I've
quickly tested this, and yes, with that we've got:

    [...]/build-gcc/./gcc/xgcc -B[...]/build-gcc/./gcc/ -xrs -nostdinc 
/dev/null -S -o /dev/null -fself-test=[...]/source-gcc/gcc/testsuite/selftests
    -fself-test: 65613 pass(es) in 0.301854 seconds

... as well as "clean" Valgrind results (just some lost memory etc., as
before).


Grüße
 Thomas


> Moral of the story is: fmemopen(3) does not allocate memory in all
> cases, RTFM
>
>>> I can easily test any patches that you need tested.
>>
>>
>> Grüße
>>   Thomas
>>
>>
>>>> On Wed, 2 Mar 2022 at 07:21, Thomas Schwinge <tho...@codesourcery.com> 
>>>> wrote:
>>>>>
>>>>> Hi!
>>>>>
>>>>> On 2022-03-02T00:15:41+0100, Mark Wielaard <m...@klomp.org> wrote:
>>>>>> On Tue, Mar 01, 2022 at 07:08:15PM +0000, 
>>>>>> build...@builder.wildebeest.org wrote:
>>>>>>> The Buildbot has detected a new failure on builder gccrust-debian-arm64 
>>>>>>> while building gccrust.
>>>>>>> Full details are available at:
>>>>>>>      https://builder.wildebeest.org/buildbot/#builders/58/builds/1710
>>>>>>>
>>>>>>> Buildbot URL: https://builder.wildebeest.org/buildbot/
>>>>>>>
>>>>>>> Worker for this Build: debian-arm64
>>>>>>>
>>>>>>> Build Reason: <unknown>
>>>>>>> Blamelist: Arthur Cohen <arthur.co...@embecosm.com>
>>>>>>>
>>>>>>> BUILD FAILED: failed compile (failure)
>>>>>>
>>>>>> And the same for all other builders.
>>>>>
>>>>> ... and me: <https://github.com/Rust-GCC/gccrs/issues/987>
>>>>> "'[...]/gcc/rust/parse/rust-cfg-parser.cc:67: rust_cfg_parser_test:
>>>>> FAIL: ASSERT_TRUE ((Rust::parse_cfg_option (input, key, value)))'".
>>>>>
>>>>>> I haven't figured out yet why the last commit caused this.
>>>>>
>>>>> (Same here.)
>>>>>
>>>>>> But it can be replicated when configuring with --enable-checking=yes
>>>>>
>>>>> That's strange -- isn't some '--enable-checking=[...]' actually the
>>>>> default for GCC builds?
>>>>>
>>>>>> That causes the selftests to trigger:
>>>>>
>>>>> At least we can see that the GCC/Rust self-tests are executing!  ;-P
>>>>>
>>>>>
>>>>> Grüße
>>>>>   Thomas
>>>>>
>>>>>
>>>>>> make[2]: Entering directory '/home/mark/build/gccrs-obj/gcc'
>>>>>> /home/mark/build/gccrs-obj/./gcc/xgcc 
>>>>>> -B/home/mark/build/gccrs-obj/./gcc/ -xrs -nostdinc /dev/null -S -o 
>>>>>> /dev/null -fself-test=/home/mark/src/gccrs/gcc/testsuite/selftests
>>>>>> rust1: error: unexpected character ‘1’
>>>>>> rust1: error: unexpected character ‘0’
>>>>>> rust1: error: unexpected character ‘0’
>>>>>> rust1: error: unexpected character ‘0’
>>>>>> rust1: error: unexpected character ‘1’
>>>>>> rust1: error: unexpected character ‘0’
>>>>>> rust1: error: unexpected character ‘0’
>>>>>> rust1: error: unexpected character ‘0’
>>>>>> rust1: error: unexpected character ‘e0’
>>>>>> rust1: error: unexpected character ‘d3’
>>>>>> rust1: error: unexpected character ‘89’
>>>>>> /home/mark/src/gccrs/gcc/rust/parse/rust-cfg-parser.cc:68: 
>>>>>> rust_cfg_parser_test: FAIL: ASSERT_EQ ((key), ("key_no_value"))
>>>>>> rust1: internal compiler error: in fail, at selftest.cc:47
>>>>>> 0x1cf096b selftest::fail(selftest::location const&, char const*)
>>>>>>        /home/mark/src/gccrs/gcc/selftest.cc:47
>>>>>> 0x7bb9a3 selftest::rust_cfg_parser_test()
>>>>>>        /home/mark/src/gccrs/gcc/rust/parse/rust-cfg-parser.cc:68
>>>>>> 0x1c143b7 selftest::run_tests()
>>>>>>        /home/mark/src/gccrs/gcc/selftest-run-tests.cc:119
>>>>>> 0xf1310b toplev::run_self_tests()
>>>>>>        /home/mark/src/gccrs/gcc/toplev.cc:2217
>>>>>> Please submit a full bug report,
>>>>>> with preprocessed source if appropriate.
>>>>>> Please include the complete backtrace with any bug report.
>>>>>> See <https://gcc.gnu.org/bugs/> for instructions.
>>>>>> make[2]: *** [/home/mark/src/gccrs/gcc/rust/Make-lang.in:232: 
>>>>>> s-selftest-rust] Error 1
>>>>>> make[2]: Leaving directory '/home/mark/build/gccrs-obj/gcc'
>>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> Mark
>>>>>>
>>>>>> --
>>>>>> Gcc-rust mailing list
>>>>>> Gcc-rust@gcc.gnu.org
>>>>>> https://gcc.gnu.org/mailman/listinfo/gcc-rust
>>>>> -----------------
>>>>> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 
>>>>> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: 
>>>>> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; 
>>>>> Registergericht München, HRB 106955
>>>>> --
>>>>> Gcc-rust mailing list
>>>>> Gcc-rust@gcc.gnu.org
>>>>> https://gcc.gnu.org/mailman/listinfo/gcc-rust
>> -----------------
>> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 
>> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: 
>> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; 
>> Registergericht München, HRB 106955
>
> Thanks a lot,
>
> --
> Arthur Cohen <arthur.co...@embecosm.com>
>
> Toolchain Engineer
>
> Embecosm GmbH
>
> Geschäftsführer: Jeremy Bennett
> Niederlassung: Nürnberg
> Handelsregister: HR-B 36368
> www.embecosm.de
>
> Fürther Str. 27
> 90429 Nürnberg
>
>
> Tel.: 091 - 128 707 040
> Fax: 091 - 128 707 077
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
-- 
Gcc-rust mailing list
Gcc-rust@gcc.gnu.org
https://gcc.gnu.org/mailman/listinfo/gcc-rust

Reply via email to