Re: Buildbot failure in Wildebeest Builder on whole buildset

[Arthur Cohen <arthur.cohen@embecosm.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 14449 bytes --]

Hi Thomas, thanks a lot!

On 3/2/22 13:36, Thomas Schwinge wrote:
> Hi Arthur!
> 
> On 2022-03-02T13:05:30+0100, Arthur Cohen <arthur.cohen@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.herron@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 knew about -wrapper but had only used it for gdb! So this is great 
information.

Weirdly, I do not get the behavior on my machine, even under valgrind.

arthur@platypus ~/G/gccrs (lexer-string-lifetime)> build/./gcc/xgcc 
-Bbuild/./gcc/ -xrs -nostdinc /dev/null -S -o /dev/null 
-fself-test=gcc/testsuite/selftests -wrapper valgrind,--leak-check=full
==87067== Memcheck, a memory error detector
==87067== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==87067== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==87067== Command: build/./gcc/rust1 /dev/null -quiet -dumpbase null 
-mtune=generic -march=x86-64 -fself-test=gcc/testsuite/selftests -o 
/dev/null -Lbuild/./gcc -L/lib/../lib64 -L/usr/lib/../lib64
==87067==
--87067-- WARNING: Serious error when reading debug info
--87067-- When reading debug info from 
/home/arthur/Git/gccrs/build/gcc/rust1:
--87067-- Can't make sense of .rodata section mapping
-fself-test: 65613 pass(es) in 7.880207 seconds
==87067==
==87067== HEAP SUMMARY:
==87067==     in use at exit: 962,793 bytes in 2,362 blocks
==87067==   total heap usage: 2,032,407 allocs, 2,030,045 frees, 
1,033,564,484 bytes allocated
==87067==
==87067== 12,640 (48 direct, 12,592 indirect) bytes in 1 blocks are 
definitely lost in loss record 1,445 of 1,454
==87067==    at 0x4845899: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==87067==    by 0x28EDF9C: ??? (in /home/arthur/Git/gccrs/build/gcc/rust1)
==87067==    by 0x4DE605F: ???
==87067==    by 0x2144E59: ??? (in /home/arthur/Git/gccrs/build/gcc/rust1)
==87067==    by 0xEB8C6B: ??? (in /home/arthur/Git/gccrs/build/gcc/rust1)
==87067==    by 0x28C452D: ??? (in /home/arthur/Git/gccrs/build/gcc/rust1)
==87067==    by 0x1F: ???
==87067==    by 0x1FFEFFFE8F: ???
==87067==    by 0x4DE6E4F: ???
==87067==    by 0x119ECDAC19DB17FF: ???
==87067==    by 0x1FFEFFFE3F: ???
==87067==    by 0x4E00C2F: ???
==87067==
==87067== LEAK SUMMARY:
==87067==    definitely lost: 48 bytes in 1 blocks
==87067==    indirectly lost: 12,592 bytes in 333 blocks
==87067==      possibly lost: 0 bytes in 0 blocks
==87067==    still reachable: 950,153 bytes in 2,028 blocks
==87067==         suppressed: 0 bytes in 0 blocks
==87067== Reachable blocks (those to which a pointer was found) are not 
shown.
==87067== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==87067==
==87067== For lists of detected and suppressed errors, rerun with: -s
==87067== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Only memory leaks, but no jumps based on uninitialized values.

There is no difference with the master branch: I kept double-checking 
since I thought I had already applied the fix.

So thanks a lot to Mark for hosting the buildbot!

> 
>> 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".  ;-)

Yes haha. But I'd like to add a little more explanation and a FIXME 
comment saying that we should think
of a better way to handle this.

>> 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).


Perfect. Thank you so much for taking the time to run it. I will open a 
pull-request for this.

I wish you all a very pleasant afternoon and thank you for helping with 
this!

Kindly,

Arthur

> 
> 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 <thomas@codesourcery.com> wrote:
>>>>>>
>>>>>> Hi!
>>>>>>
>>>>>> On 2022-03-02T00:15:41+0100, Mark Wielaard <mark@klomp.org> wrote:
>>>>>>> On Tue, Mar 01, 2022 at 07:08:15PM +0000, buildbot@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.cohen@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.cohen@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


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3195 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]