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 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 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, Rust::Lexer::TokenSource>::peek(int) (rust-lex.h:233) >>> ==3228208== by 0x988A46: Rust::parse_cfg_option(std::__cxx11::basic_string, std::allocator >&, std::__cxx11::basic_string, std::allocator >&, std::__cxx11::basic_string, std::allocator >&) (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 wrote: >>>>>> >>>>>> Hi! >>>>>> >>>>>> On 2022-03-02T00:15:41+0100, Mark Wielaard 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: >>>>>>>> Blamelist: Arthur Cohen >>>>>>>> >>>>>>>> BUILD FAILED: failed compile (failure) >>>>>>> >>>>>>> And the same for all other builders. >>>>>> >>>>>> ... and me: >>>>>> "'[...]/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 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 >> >> 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