- User Since
- Aug 20 2018, 1:38 PM (267 w, 1 d)
Wed, Sep 20
Are you able to post the test source or an unoptimized reproducer?
Actually, I think that expression might be invalid? That argument 1 is supposed to mimic the DWARF specification, i.e., it attempts to specify the length of the operations that are evaluated upon function entry. We require it to be 1 because we always want to be just a "register" operation. It's not clear what the meaning would be for a variadic expression
I've just noticed that, for reasons I am still investigating, the optimizer produced this expression in some test:
Mon, Sep 18
Thu, Sep 14
Thanks for the explanation, it makes sense!
Wed, Sep 13
However, I've found the patch triggers some errors when running on the llvm test suite, and the variadic check really isn't that expensive I think, so I'm going to rewrite the patch to do a bit more.
Tue, Sep 12
Mon, Sep 11
I think this may have broken builds:
Fri, Sep 8
Fix incorrect comment in commit message.
Thu, Sep 7
Since this is DWARF5/ObjectiveC only, and since it resolves more failures than it creates, I'll go ahead and merge it.
This is exposing an issue with lldb/test/API/lang/objc/modules-objc-property/TestModulesObjCProperty.py.
This test compiles an ObjectiveC program and runs dsymutil on the binary.
By default, dsymutil verifies the output only if the input is valid.
With this patch, the input is now considered valid. So dsymutil verifies the output, which turns out to be invalid (for other reasons?)
If I don't apply this patch and force dsymutil to verify the output of that test, it also says the output is invalid.
Just to register in case there are any concerns with performance here, I built Clang in Debug, generated a dsym with dsymutil build_Debug/bin/clang --accelerator Dwarf and measured the time it takes to verify it: build_Release/bin/llvm-dwarfdump --verify build_Debug/bin/clang.dSYM
Address review comments
Wed, Sep 6
Address review comments
Hey Scott, do you think it will be possible to merge these patches soon-ish? I would like to make some further changes to the entry value side of things, and it would be nice to get this in before.
Aug 31 2023
Aug 29 2023
I decided to make it a global object to guarantee its lifetime independent of any PlatformRemoteGDBServer instance. I don't mind wrapping it in a concurrent structure to guarantee thread safety though.
I am slightly wary of making this a global set without any kind of thread safe mechanism, as I (naively, not really knowing how this class is used) would expect us to be able to instantiate multiple servers and have them be accessed concurrently. As such, it makes more sense to have each server own its set of strings.
Aug 28 2023
Remove duplicate line
Make a test run the verifier.
Aug 25 2023
Really like the code deletions here :)
LGTM! Thanks for doing this
Everything looks good to me, this is much cleaner! On my first encounter with this class, it took me a long time to decipher what was going on, and the variant design is way more expressive.
Thank you so much for doing this! I was about to start working on this, so I'm thankful you posted this when you did! I'll have a look now
Aug 24 2023
Argh, I added one more test along the same lines, gotta fix that one too
Pushed a quick fix
@StephenTozer have you received any other feedback on this? I've been experimenting with this patch in the swift fork (https://github.com/apple/llvm-project) and it seems to fix all the issues I had found.
Apparently this is failing to link on the nvidia MLIR bot. Investigating
Aug 23 2023
Reword a comment