User Details
- User Since
- Feb 9 2017, 3:56 PM (287 w, 3 d)
Tue, Aug 9
Mon, Aug 8
Mon, Jul 25
Some more information about the problem. The stack trace looks like follows. The problem manifests as an ASan out of memory error because the move constructor in varlen_sort.h:59 is getting incorrect data and it's trying to allocate an absurdly big amount of memory.
Thu, Jul 21
Removed unnecessary parentheses.
Wed, Jul 20
Tue, Jul 19
Looks good to me.
Jul 13 2022
After this patch, llvm-dis will crash if passed a bitcode file with metadata that is now non-representable.
Jul 12 2022
Jun 27 2022
Yes, it fixes both the original compilation where we noticed the problem
and the reduced test case. Thank you very much for the fix and the quick
response!
Jun 7 2022
I've been experimenting a little bit and it seems that I can avoid the deadlocks I was seeing by applying only the modification to TypeCategoryMap::Add. This would avoid the problem that @hawkinsw pointed out in my change to FormatManager::GetCategoryForLanguage. However, the following problem could still happen with the change to TypeCategoryMap::Add:
Jun 3 2022
Jun 2 2022
May 23 2022
May 20 2022
May 19 2022
May 4 2022
Apr 19 2022
Apr 14 2022
Apr 11 2022
Thanks for the reply! Yes, I've seen some test failures while trying to update our llvm to (closer to) HEAD, and I bisected them to this commit. These were the affected tests:
Apr 8 2022
Is it possible that this change just broke the lldb data formatter for string?
Apr 7 2022
Apr 5 2022
Mar 31 2022
By the way, the line number for llvm::misexpect::verifyMisExpect in the stack trace above includes the debug printfs I inserted to get the variable values so it won't match the exact line number in the repo. But I think that's the only call to BranchProbability in that function so I hope it's not very confusing anyway. Sorry about that.
Hi, this patch is causing floating point exceptions for us during profile generation. On a debug build I get this assertion failure (see stack trace below):
clang: /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/BranchProbability.cpp:41: llvm::BranchProbability::BranchProbability(uint32_t, uint32_t): Assertion `Denominator > 0 && "Denominator cannot be 0!"' failed.
Mar 21 2022
We've observed that this patch introduces infinite loops in some cases. Here's a reduced test case:
export class Foo extends Bar { get case(): Case { return ( (this.Bar$has('case')) ? (this.Bar$get('case')) : (this.case = new Case())); } }
Mar 4 2022
We've found segfaults in lld when linking go binaries after this patch:
Mar 1 2022
Feb 28 2022
Nov 11 2021
This breaks some existing code (we have found this issue building the JPEG XL library at github.com/libjxl). This is a very simplified version of the problem:
Nov 10 2021
I rebuilt clang at this commit with -DLLVM_USE_SANITIZER=Address and it reports a problem nearby. I don't know if this patch is the actual root cause for the crash or if it's triggering an existing bug. Here's the full report in case it can help you debug the issue further:
Hi, I created the reproducer that akuegel posted above. I have reproduced the crash starting with a clean repo with the steps below. The stack trace is a bit different, but a release build following the same steps will get you a stack trace very similar to what he posted. What I've noticed is that the crash does not always reproduce consistently. I was trying a new release build to double check the steps below, I ran the build command multiple times in a row, and it crashed only sometimes. The crash is a segfault, so it's possible that sometimes an invalid memory access doesn't immediatly crash the program.
Nov 9 2021
This change is causing asan errors when running the clang-tidy checks under ASan, most likely because of the reason akuegel pointed out in his comment. I'm going to revert the patch to unbreak HEAD until the problem can be fixed.
Jun 17 2021
May 5 2021
May 3 2021
Hi, we're seeing some build failures after this patch. If you build the following reduced test case with clang -cc1 -emit-obj -O3 -vectorize-slp reduced.cc
Apr 21 2021
Hi, one of our tests is failing because of this patch and it looks like an actual bug. clang-format now turns this file:
import './a'; import {bar} from './a';
into this:
barimport './a';
Feb 23 2021
I'm going to go ahead and commit this given that I have just made the last suggested modification and the patch has already been up for review without further comments for a long while (sorry for the late replies, work keeps getting in the way of work).
Changed logic that checked the type of unit to use header.IsTypeUnit() as suggested by reviewer.
Feb 11 2021
Moved index handling to DWARFUnit.cpp as suggested by Pavel.
Feb 5 2021
Jan 5 2021
Thanks for accepting the patch! (and sorry for the late reply, I was on vacation). I'll commit this now :)
Dec 16 2020
Dec 14 2020
Hi! We have bisected a clang crash to this patch. The following reduced test case crashes clang when built with clang -O1 -frounding-math:
Dec 3 2020
Found a crash that seems to be caused by this patch. This is a reduced test case:
extern "C" #define a(b, c, args) d(double, b, , args) #define d(g, b, c, args) g b args a(sqrt, , (double)); int e; double f = sqrt(e);
Building with clang -target powerpc64le-grtev4-linux-gnu -ffast-math repro.cpp crashes a top-of-tree clang.
Nov 19 2020
Thanks everyone for the reviews! I'm committing this now.
Nov 18 2020
then the check Rows[H].getSignature() != S won't fail
The condition fails. It should return nullptr but returned a Rows entry.
Updated diff addressing the new round of feedback.
Addressed reviewer comments.
Nov 17 2020
Thanks for the comments!
Changed test expectation from "not a stack dump" to the actual output from llvm-symbolizer for the test case.
Aug 18 2020
LGTM
Aug 10 2020
This looks good to me FWIW. I'd prefer if someone more experienced with the project would accept it, though.
Aug 6 2020
Apr 7 2020
Mar 17 2020
Thanks for the note about the flags. We have tried both of them, and any (or both) of them being present causes the regression to go away, so that unblocks us while we work on producing a proper test case.
Mar 16 2020
We're seeing some large performance regressions on Eigen (http://eigen.tuxfamily.org/) running on haswell and skylake machines with this patch. Could you please revert it?
Mar 12 2020
Mar 11 2020
Thanks for doing the extra work of adding a test. Just one final nit.
I think it looks good now. My only issue is that it seems to rely on a couple of glibc-specific features: glibc modifying the fields adds and subs in dl_phdr_info when loading/unloading libraries (which this patch uses to know when to invalidate the cache), and dl_iterate_phdr holding a lock (which the patch relies on to avoid races while accessing the cache). What other libc implementations do we support? Do they share these behaviors we rely on here?
Mar 10 2020
Mar 9 2020
Thanks!
Looks good to me.