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.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 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 faultHeh, 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 anyThat 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 ThomasMoral of the story is: fmemopen(3) does not allocate memory in all cases, RTFMI can easily test any patches that you need tested.Grüße ThomasOn 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=yesThat'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 Thomasmake[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 106955Thanks 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
OpenPGP_0x1B3465B044AD9C65.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
-- Gcc-rust mailing list Gcc-rust@gcc.gnu.org https://gcc.gnu.org/mailman/listinfo/gcc-rust
