-
Notifications
You must be signed in to change notification settings - Fork 13.3k
debuginfo: Fix gdbr-check expectations of formatting #40351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This was a largely automated process, driven by the hack of a shell script below. The process to generate the patch was to repeatedly run the script until the changes reached a fixed-point. ```bash set -euo pipefail O=$(mktemp) ./x.py test src/test/debuginfo 2>&1 | tee $O || true awk ' function escape_pattern(pat, safe) { safe = pat gsub(/[][^$.*?+{}\\()|]/, "\\\\&", safe) return safe } BEGIN { p = 0; n = 0; } /---- \[debuginfo-gdb\]/ { if (p == 1) { printf("%s|%s|%s\n", src, error, expected); } src = $3; p = 1; n = 0; name = ""; } n == 1 && $0 ~ name { expected = $0; n = 0; } /error: line not found in debugger output/ { error = substr($0,43); split(error, pieces); name = escape_pattern(pieces[1]); print "Found " name " in: " $0 > "/dev/stderr" n = 1; } END { printf("%s|%s|%s\n", src, error, expected); } ' $O | while read l; do SRC="$(echo "$l" | cut -d'|' -f1 | sed 's/debuginfo-gdb/debuginfo/')" ERR="$(echo "$l" | cut -d'|' -f2)" V="$(echo "$ERR" | cut -d' ' -f1 | sed 's/\$/\\$/g')" EX="$(echo "$l" | cut -d'|' -f3)" if echo "$EX" | grep '^\$' > /dev/null; then echo $SRC sed -ri '/\/\/ gdbr-check:'"$V"' /s|(// gdbr-check:).*|\1'"$EX"'|' src/test/$SRC else # this is a bit questionable EX="$(echo "$l" | cut -d'|' -f2)" sed -ri '/\/\/ gdbr-check:'"$V"' /s|(// gdbr-check:).*|\1'"$EX"'|' src/test/$SRC echo $SRC: Failed to patch with: $l fi done echo $O ``` Fixes: rust-lang#39838 Signed-off-by: Andrew Jeffery <[email protected]>
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
First of all thanks for the PR @amboar! A few notes:
|
So I just checked with The output of
|
Yeah, it's curious. My interest is more in understanding what is going on than necessarily getting this PR merged in its current form. I still get the failures on recent Rust master (b04ebef) running in a Ubuntu Yakkety environment, with both the Ubuntu system @TimNN what environment are you testing in? For the record the diff between the system and local gdb configurations is minimal: $ diff -u <(/usr/bin/gdb --configuration) <(/usr/local/bin/gdb --configuration) [6/1883]
--- /dev/fd/63 2017-03-10 15:21:43.586582443 +1100
+++ /dev/fd/62 2017-03-10 15:21:43.586582443 +1100
@@ -1,16 +1,15 @@
This GDB was configured as follows:
- configure --host=x86_64-linux-gnu --target=x86_64-linux-gnu
+ configure --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu
--with-auto-load-dir=$debugdir:$datadir/auto-load
--with-auto-load-safe-path=$debugdir:$datadir/auto-load
--with-expat
- --with-gdb-datadir=/usr/share/gdb (relocatable)
- --with-jit-reader-dir=/usr/lib/gdb (relocatable)
+ --with-gdb-datadir=/usr/local/share/gdb (relocatable)
+ --with-jit-reader-dir=/usr/local/lib/gdb (relocatable)
--without-libunwind-ia64
--with-lzma
- --with-python=/usr (relocatable)
+ --with-python=/usr
--without-guile
- --with-separate-debug-dir=/usr/lib/debug (relocatable)
- --with-system-gdbinit=/etc/gdb/gdbinit
+ --with-separate-debug-dir=/usr/local/lib/debug (relocatable)
--with-babeltrace Probably not surprising given they're both failing without the patch in the PR.
So for me, without the patch in the PR the tests fail when the pretty printers from @TimNN it'd be interesting to know if your gdb was confirmed to load the files and pass the tests. In which case there's something else at play that should be tracked down. |
For most of the debuginfo tests, gdb is not supposed to load any pretty printers, they all use I think the interesting question now is why your gdb does load the pretty printers. |
I think I ran into this problem a while ago, but I can't remember the details. A couple of things to check:
|
I get |
I have just reproduced this while investigating something unrelated, without further investigation I think this is related to either: a) gdb being compiled for multiple targets or @amboar: Did you do one of those two things? |
Sorry guys, I was trying to get back to this today but other things have intervened so far. Hoping to get time in the next few hours. |
@TimNN I built two configurations:
This passed the debuginfo tests. On the other-hand,
fails the tests. I haven't looked into why yet but at least on ppc64 we need to run
I doubt |
I don't have a build to check on right now, but probably the debug libraries include the |
@philipc spot on! From the release build:
From the debug build:
|
You could try filtering out the symbol name of the only symbol in the |
@michaelwoerister Thanks - I was considering something along those lines but hadn't looked at where to start. I'll follow your leads when I find some time to get back to the issue but it might not be for a week or so. If someone wants to pick it up in the mean time they should go for it. |
I'm going to close out this PR for the time being, just to clear up the queue. @amboar, feel free to reopen (or open a new one) when you have a chance to try out @michaelwoerister's idea. |
@aturon No worries |
This pull-req is one of the solutions outlined in #39838. There's a fairly detailed run-down there that I won't repeat here. I'm not sure which way the true solution is meant to go, so I'm creating a pull-req to give it a bit more attention.