- User Since
- Jun 3 2014, 6:35 AM (328 w, 5 d)
Mon, Aug 31
Specifically, I don't want to mess with the setup for Android and/or non-Darwin crosscompiling, because I have no way of testing that.
Also, I understand that the code in question isn't best already, but I don't have the bandwidth to refactor and test any more extensive changes. I'm only trying to address the problem of getting -arch arm64 / arm64e passed into tests, otherwise we're testing the wrong thing. Let's focus on that, please.
@dvyukov Any feedback here, or is this OK to go in?
Sun, Aug 30
Looks good for get rid of the test failure.
Tue, Aug 25
Great finds, thanks! I've updated the missing places. I'm leaving the Go parts alone, for now, as Go isn't using ptrauth.
Fri, Aug 21
Definitely agree that this is very annoying to read. I think it would be best if we were able to change the ifdef to use something like “HAS_48_BIT_ADDRESS_SPACE” and compute that bool separately. @yln, @dvyukov WDYT?
Ah, sorry :) The context is that any test config for arm64 or arm64e isn’t getting the right -arch flag passed in.
Aug 19 2020
This looks reasonable, but just for verification, could you post the "before" and "after" of the little shadow map printout from ASAN_OPTIONS=verbosity=1?
Aug 18 2020
Aug 17 2020
If we use #if !SANITIZER_MAC || defined(__MAC_10_14), do we still need the forward declarations and weak_import stuff?
Aug 14 2020
This looks good to me. Note that I was actually suggesting to avoid the dynamic_lookup + forward declare and requiring that the *SDK* is at least 10.14 to run tests (which is separate from your OS version). But this way it's even more compatible, so I think we should go with it.
Aug 13 2020
LGTM. Maybe add an if to check if the 10.14-only API is available and just do nothing in the test otherwise?
Does the testcase actually fail without the code change?
Aug 4 2020
If I read the output right, it seems that even before this change, running with TSan messes up the permissions argument. I think this may be some latent bug in glibc that is only triggered in unusual environments (like TSan). I found this comment at https://lkml.org/lkml/2014/10/30/812:
Aug 1 2020
...and could you also see if perhaps this change helps:
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp --- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp @@ -137,7 +137,11 @@ const int PTHREAD_BARRIER_SERIAL_THREAD = -1; #endif const int MAP_FIXED = 0x10; typedef long long_t; +#if SANITIZER_MAC || SANITIZER_FREEBSD || SANITIZER_NETBSD typedef __sanitizer::u16 mode_t; +#else +typedef __sanitizer::u32 mode_t; +#endif
Jul 30 2020
I can provide you access to a PPC64LE machine or I can check the details myself if you can tell me exactly what to do and how.
Would anyone with access to a Linux/PPC64 machine be willing to take a look if the open() syscall is receiving the right value (0600 from the testcase)?
Actually given the symptom, I'm thinking this could be a real problem with the variadic interceptor:
Jul 29 2020
Jul 24 2020
Jul 21 2020
Let's handle simulators separately.
Jul 17 2020
Jul 14 2020
Jun 30 2020
Looks reasonable. Could you check that swift_demangle has been correctly returning NULL for a "long time" (a couple years at least)? I'm asking because we can run on older OS's.
May 1 2020
Overall LGTM - would be great to hear feedback from @eugenis though.
Apr 20 2020
Apr 17 2020
Apr 15 2020
Apr 8 2020
Apr 6 2020
Mar 18 2020
Mar 5 2020
Jan 8 2020
I've mentioned this to Julian in person: The code change look good to me, but any changes in this piece of code need a high level of manual testing as this code is very critical and has potential for breaking something even with innocent changes like this.
Dec 16 2019
Oct 7 2019
Aug 28 2019
Aug 27 2019
Aug 26 2019
Looks good to me. @dvyukov ?
Aug 23 2019
Aug 14 2019
LGTM, even if it's just for Darwin now.
Aug 9 2019
Aug 1 2019
Jul 30 2019
Thanks! This looks like a great step in the right direction. An overall question first: Should we use posix_spawn for *all platforms*? IIRC, unifying the launch paths was discussed on mailing lists before and sounds desirable.
Jul 24 2019
May 22 2019
Looks like there's no reviewers set here. @t.p.northover who should review this?
May 8 2019
Nice, thanks. I don’t understand the ELF code so someone else should review it, but from my view this LGTM.
Apr 17 2019
Okay. @jingham, any more comments, or is this okay to land?
Apr 16 2019
Sorry for the huge delay. @yln can you test this patch (that is passes all tests) and if yes, land it?
Updating test to use run_to_source_breakpoint
Adding a test case.
Apr 15 2019
Apr 12 2019
I like this change, but there's no reason to apply it specifically to the Darwin tests, right? Can you apply it to all TSan tests?