User Details
- User Since
- Jan 2 2013, 4:34 PM (419 w, 4 d)
Fri, Jan 15
lgtm
I see. Maybe ASLR matters? I have the full stack trace if it helps:
https://reviews.llvm.org/P8253
- fix misleading comment
CReduce on plain C code is lightning fast, no template goo grind away. I got this reduced test case that crashes when this patch is applied:
I reverted this, it caused crashes while compiling harfbuzz in Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1167305
Thu, Jan 14
Thanks, I think this will solve the issue.
lgtm
Thanks, this seems like the right spot.
Wed, Jan 13
I don't see how supporting this attribute would break interop with MSVC. MSVC doesn't support these attributes, only GCC and Clang do. Presumably in the wine environment, some headers using these _GNUC extensions are being included somewhere, and IMO we should accept the attributes and honor them.
This actually caused https://crbug.com/1166386. The explanation is that defining _LIBCPP_DEBUG causes _LIBCPP_EXTERN_TEMPLATE to be defined to nothing, which disables the extern declarations. Chrome uses _LIBCPP_DEBUG in debug builds. IIUC, locale needs these extern template declarations, because locale uses static data members with hidden visibility (need to confirm). If you remove the extern template declarations, then duplicate static data members may be emitted into the user's DSO, and when they use locale facets, they will be registered with a new id. I need to dig more to confirm, but this is what I'm going on.
Please add a lit test for this. The test could actually copy llvm-symbolizer into a temporary directory, and you could embed the option in __asan_default_options similar to what we want to do in Chromium. I think it's also reasonable to test that if llvm-symbolizer doesn't exist, we get a warning, not an error.
lgtm
Tue, Jan 12
Thanks, I'll push this after a few more tests.
lgtm, thanks!
Mon, Jan 11
lgtm, thanks
lgtm
We are also observing this build breakage, so I'm going to go ahead and push this now.
Fri, Jan 8
I put some comments on https://bugs.llvm.org/show_bug.cgi?id=48432#c4 about this. I'm not sure we should keep going this direction. Thanks for getting numbers, though.
Please fix the two test failures observed in the harbormaster output. Your change will cause these tools to produce CRLF output, which tests must account for.
Thu, Jan 7
lgtm with nit
lgtm
- formatting
lgtm
lgtm
Wed, Jan 6
I guess a triple of -fuchsia-itanium would be a reasonable way of expressing this.
Generally makes sense, but I had a concern.
lgtm
Tue, Jan 5
- Remove RSP from CSR lists too
Reposting my comment here, since this is where the discussion was:
I did some comparisons of the sanitizer object files, and I think this change broke -gline-tables-only functionality. The new object file had a large .debug_loc section, which is only present when full debug info is enabled, and is not desired in this case.
The Harbormaster precommit test stuff is complaining, but maybe it's just because of the binary files in the patch.
It seems like Harbormaster / Jenkins didn't like the new test, so double check that it passes before landing.
lgtm
I'm not an SCC expert, but Arthur explained to me how this works, and I trust the explanation.
lgtm
Mon, Jan 4
Thank for the update, apologies for not providing these suggestions the first time.
Sat, Dec 19
Dec 17 2020
lgtm
Dec 16 2020
lgtm, the comment is a suggestion
lgtm
lgtm
Dec 15 2020
Thanks for the fix. At first, misunderstood, I expected that an aggregate containing non-aggregates should be returned indirectly, and that the fix would be in the C++ ABI codepath. However, I see that is not the case. An aggregate may contain non-aggregates, and MSVC will in fact return such a type directly in X0/X1.