| Summary: | valgrind.h doesn't compile on s390x with clang | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Avi Kivity <avi> |
| Component: | general | Assignee: | Andreas Arnez <arnez> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | arnez, jonathan.albrecht, pjfloyd |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
This seems very similar to https://bugs.kde.org/show_bug.cgi?id=342008 which just added an explicit cast for _zzq_default. I tried the same fix for s390x with: diff --git a/include/valgrind.h b/include/valgrind.h index d33dd3093..04a747c7a 100644 --- a/include/valgrind.h +++ b/include/valgrind.h @@ -876,7 +876,8 @@ typedef /* results = r3 */ \ "lgr %0, 3\n\t" \ : "=d" (_zzq_result) \ - : "a" (&_zzq_args[0]), "0" (_zzq_default) \ + : "a" (&_zzq_args[0]), \ + "0" ((unsigned long int)_zzq_default) \ : "cc", "2", "3", "memory" \ ); \ _zzq_result; \ and it fixes the compile error but I don't know if that's sufficient. FWIW, I built valgrind from source using gcc and ran the reg tests both with and without the change and got the same results. (In reply to Jonathan Albrecht from comment #1) > [...] > I tried the same fix for s390x with: > > diff --git a/include/valgrind.h b/include/valgrind.h > index d33dd3093..04a747c7a 100644 > [...] FWIW, the patch looks OK to me, and I don't see why it shouldn't be sufficient for fixing this error. @Julian: Isn't the patch necessary for other 64-bit platforms as well then? Such as ppc64le or amd64? There is certainly no problem on FreeBSD. Just double checked with clang 10 and clang 12 (don't have 11 handy). I have done builds with clang on Fedora and I don't remember seeing any such issues. Will confirm tomorrow. (In reply to Andreas Arnez from comment #2) @Andreas: if the patch works on s390, I think you should land it. > @Julian: Isn't the patch necessary for other 64-bit platforms as well then? > Such as ppc64le or amd64? Well, we have no reports of such problems on either ppc64le or amd64. Though looking at the code, I fundamentally agree with you .. I don't see why s390 should have a problem here but not the others. Quick confirmation for amd64 Fedora. No problems with clang 11 or clang 13 (built from git head). In fact, even with -Weverything I only get a few warnings about the use of leading double underscores. That is pretty good as -Weverything often generates many reams of warnings. (In reply to Paul Floyd from comment #5) > Quick confirmation for amd64 Fedora. No problems with clang 11 or clang 13 Thanks. (general comment, not suggesting any action right now): Looking at valgrind.h now, I am thinking that it is a mistake for us to use `unsigned long int` as the basic machine-word type for 64-bit targets. Perhaps we should change those en masse to `unsigned long long int`, so as to avoid any ambiguity in the type-sizes. TBH I am not sure why it uses `unsigned long int` at all. (In reply to Julian Seward from comment #4) > @Andreas: if the patch works on s390, I think you should land it. Done. Pushed as 484b7dd1e862b1624cb8e7aa453df277da4f7a15. Maybe thanks for fixing the bug. Please consider a patch release so I can lobby my distribution for updating its valgrind. (Many and unconditional thanks, not maybe thanks). Valgrind 3.17.0 has been released, and it contains the fix for this Bug. See the announcement: https://sourceforge.net/p/valgrind/mailman/message/37246072/ |
SUMMARY Given a file test.c with the content ---8<--begin content---8<---- #include <valgrind/valgrind.h> ---8<---end content----8<---- It fails to compile on s390x: clang -o test.o test.c In file included from test.c:1: /usr/include/valgrind/valgrind.h:6768:47: error: unsupported inline asm: input with type 'int' matching output with type 'volatile unsigned long' _qzz_res = VALGRIND_DO_CLIENT_REQUEST_EXPR(0, ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ /usr/include/valgrind/valgrind.h:879:49: note: expanded from macro 'VALGRIND_DO_CLIENT_REQUEST_EXPR' : "a" (&_zzq_args[0]), "0" (_zzq_default) \ ^~~~~~~~~~~~ /usr/include/valgrind/valgrind.h:6807:47: error: unsupported inline asm: input with type 'int' matching output with type 'volatile unsigned long' _qzz_res = VALGRIND_DO_CLIENT_REQUEST_EXPR(0, ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ /usr/include/valgrind/valgrind.h:879:49: note: expanded from macro 'VALGRIND_DO_CLIENT_REQUEST_EXPR' : "a" (&_zzq_args[0]), "0" (_zzq_default) \ ^~~~~~~~~~~~ 2 errors generated. STEPS TO REPRODUCE 1. Create a file with the provided content 2. Compile it on s390x with clang OBSERVED RESULT Errors EXPECTED RESULT No errors SOFTWARE/OS VERSIONS Linux, Fedora 33, valgrind-3.16.1-8.fc33.s390x, clang-11.0.0-2.fc33.s390x ADDITIONAL INFORMATION