- User Since
- Sep 4 2014, 1:00 AM (354 w, 4 d)
Apr 28 2021
Apr 22 2021
Apr 6 2021
One little comment: should one of the tests where you removed the pubnames testing actually be replaced a CHECK-NOT: .debug_pubnames contents: ?
Mar 5 2021
Feb 24 2021
Jan 12 2021
Jan 11 2021
Couple comments/questions, but this generally looks good
Dec 10 2020
There is one thing that you might want to address, but I'll leave it up to you (and even if you do it can be a different commit): with the introduction of idx variables, it becomes easy to get confused between id and idx(Or CheckID and CheckIndex) in the codebase. Not sure how to disambiguate, or if it's really worth it.
Dec 9 2020
The change itself looks fine, but I'm wondering about the test. There is nothing that really guarantees that the SB calls and return values are going to be intertwined, is there? Does it rely solely on the fact that it's very very unlikely to succeed?
This LGTM. I like that it's way easier than what we have discussed offline.
Nov 6 2020
This is really hard to wrap one's head around, but after starring at it for some time the logic seems sound. There's some conditional code depending on whether we built with -gmodules or not. Can you make sure to test both clang builds? I left a couple other comments that you might want to consider. Otherwise this LGTM
Oct 14 2020
Aug 26 2020
Aug 12 2020
Aug 7 2020
Sure. As I said I know this is unrelated to the path. this LGTM
lldbremote.py looks awfully Apple-specific, yet it is going to be called from the generic code. Is that not an issue?
This LGTM. The only comment I had is about a comment you added on preexisting code:
Aug 6 2020
Aug 5 2020
Aug 4 2020
The logic of the patch itself looks fine, but the names, description and commit message are off. A process cannot disable TCC, it's always active. What your patch decides is whether the inferior is responsible for its own TCC permissions. If you don't make the inferior responsible, it inherits the permissions of its parent (which might have inherited them from its own parent). On the command line, you'll most likely get the Terminal.app permissions if nothing in the middle resets the responsible process.
Aug 3 2020
This looks good outside of 2 cross-platform issues in the tests themselves.
Jul 27 2020
Jul 24 2020
Simplify the logic.
Jul 23 2020
Jul 21 2020
Jul 20 2020
The lldbtest.py part LGTM, but I'm failing to see how this interacts with TestWeakSymbols.py? IIRC, registerSharedLibraryWithTarget is an API we call explicitly in the tests, but I don't see it called here.
Jul 17 2020
Jul 16 2020
I'll commit after I've re-built top of tree and fully retested
Address review feedback
Jul 15 2020
- Rebase on top of D83512
- Change the ObjectFileMachO pieces to rewrite offsets to look like a standard Mach-o image instead of adding a bunch of conditionals to handle the new cases.