[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM. I'll commit for you once Greg reviews it again.



================
Comment at: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c:1
+__attribute__((section("__codesection")))
+int f(int a) {
----------------
kbaladurin wrote:
> clayborg wrote:
> > Will this work with all compilers we currently run the test suite with? I would assume with will work with GCC and Clang at least. IF not, we might need to make a lldbtest.h file that any test case can use and use a macro here?
> What compilers do we use with test suite? This construction works fine with gcc and clang. Is it enough for us?
I think this is fine.
BTW, if we really want to abstract this, the place is not lldb but llvm.
We already have an header for the purpose.

```
llvm/Support/Compiler.h
```

But again, I don't think this is needed.


================
Comment at: source/Core/Section.cpp:30
 
-static const char *GetSectionTypeAsCString(lldb::SectionType sect_type) {
+const char *Section::GetSectionTypeAsCString(lldb::SectionType sect_type) {
   switch (sect_type) {
----------------
kbaladurin wrote:
> clayborg wrote:
> > Why did you take static off of this function? Please remove this change, or change this function to get the section type from the section itself and not require the argument.
> I change it to static method to use it in `lldb-test`. There is similar static methods in `Value` and `Scalar` classes: `Value::GetValueTypeAsCString` and `Scalar::GetValueTypeAsCString`. Is non static method more preferable for us in this case?
I think what you did was correct. Greg?


https://reviews.llvm.org/D44998