[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-04 Thread António Afonso via Phabricator via lldb-commits
aadsm added a comment. Ok. Initially I thought that with the entry point we were making big assumptions there but after reading the parse unwind symbols I guess it's really no big difference. I guess my main concern is that the user can no longer create symbols within the span of the entry

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-04 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Yea, I see what you mean. I wouldn't spend too much time on fixing that though. The ability for SymbolFiles to add symtab entries is a fairly new thing. Not all issues with it have been ironed out, and I don't think it's up to you to fix them. The same kind of conflict

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-03 Thread António Afonso via Phabricator via lldb-commits
aadsm added a comment. I ended reverting this because of SymbolFile/Breakpad/symtab.test. It seems that in this test we add symbols after the synthetic entry point symbol is added creating confusion. I think I'll need to change the code that adds symbols in Symtab to explicitly check if we're

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-03 Thread António Afonso via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373680: Explicitly set entry point arch when its thumb (authored by aadsm, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-02 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 222967. aadsm added a comment. Remove .global _Start Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68069/new/ https://reviews.llvm.org/D68069 Files: lldb/lit/SymbolFile/dissassemble-entry-point.s

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-02 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/lit/SymbolFile/dissassemble-entry-point.s:10 + +.global _Start +_start: This .global _Start is also unneeded, as it does not even match

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-02 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 222848. aadsm added a comment. Update comment and test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68069/new/ https://reviews.llvm.org/D68069 Files: lldb/lit/SymbolFile/dissassemble-entry-point.s

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-02 Thread António Afonso via Phabricator via lldb-commits
aadsm added a comment. I actually just completely missed the first comment on the .s test and forgot about the comment . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68069/new/ https://reviews.llvm.org/D68069

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-02 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks. The code looks fine now, but it doesn't look like you've addressed my comments in the test (or at least, they didn't make it into the uploaded version). Also, I don't think the long comment before the code in ObjectFileELF really reflects what the code does

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-01 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 222680. aadsm added a comment. Update tests and fix logic to correctly add the symtab entry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68069/new/ https://reviews.llvm.org/D68069 Files:

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-10-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks for adding the lit test. Comment at: lldb/lit/SymbolFile/dissassemble-entry-point.s:6 +# RUN: %lldb -x -b \ +# RUN: -o 'settings set disassembly-format "{ <${function.concrete-only-addr-offset-no-padding}>}: "' \ +# RUN: -o 'dis -s 0x8074 -e

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-30 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 222490. aadsm added a comment. Add lit test to check dissassembly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68069/new/ https://reviews.llvm.org/D68069 Files: lldb/lit/SymbolFile/dissassemble-entry-point.s

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D68069#1686199 , @aadsm wrote: > > Not everyone has both of these things enabled (particularly having lld is > > less common), but they are things that one _can_ enable no matter what is > > his host or target architecture,

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-27 Thread António Afonso via Phabricator via lldb-commits
aadsm added a comment. > (substituting llvm-mc for as and ld.lld for linker). I was not able to figure out how to generate the object file with llvm-mc correctly. I've tried a few variations of the triple (e.g.: `armv7-eabi`) but I never end up with the same code that I have in the assembly:

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-27 Thread António Afonso via Phabricator via lldb-commits
aadsm added a comment. > Not everyone has both of these things enabled (particularly having lld is > less common), but they are things that one _can_ enable no matter what is his > host or target architecture, so I am not worried by that. And as tests like > these become more common, I

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D68069#1685146 , @aadsm wrote: > > For the test, what would you say to writing that as a lit test instead > > (testing the address class deduction via the disassemble command you > > mentioned)? > > I was actually keen on this

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-26 Thread António Afonso via Phabricator via lldb-commits
aadsm updated this revision to Diff 222063. aadsm added a comment. Use GetNextSyntheticSymbolName() to generate the symbol name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68069/new/ https://reviews.llvm.org/D68069 Files:

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-26 Thread António Afonso via Phabricator via lldb-commits
aadsm marked an inline comment as done. aadsm added a comment. > For the test, what would you say to writing that as a lit test instead > (testing the address class deduction via the disassemble command you > mentioned)? I was actually keen on this since lit is the only type of test I haven't

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-26 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. This sounds like a reasonable thing to do, but it seems to me we make that more generic. Having a symbol for the entry point would help in other cases too, and we already go through a lot of trouble to track down various symbol addresses (plt parsing, eh_frame parsing,

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

2019-09-25 Thread António Afonso via Phabricator via lldb-commits
aadsm created this revision. aadsm added reviewers: clayborg, labath, wallace. Herald added subscribers: lldb-commits, MaskRay, kristof.beyls, arichardson, emaste, srhines. Herald added a reviewer: espindola. Herald added a project: LLDB. I found a case where the main android binary