[Lldb-commits] [PATCH] D16563: Set symbol types for function symbols loaded from PE/COFF

[Zachary Turner via lldb-commits]

zturner added inline comments.

================
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:183
@@ +182,3 @@
+    }
+    return lldb::eSymbolTypeInvalid;
+}
----------------
amccarth wrote:
> zturner wrote:
> > Instead of returning `eSymbolTypeInvalid` here, how about `eSymbolTypeData`?  If you look in `llvm/Support/COFF.h` all of the non function types are data.
> I'm not sure about that.  Some of the symbols are clearly sections (.text, etc.).  Those have 0 for the COFF type, which COFF.h says means "No type information or unknown base type."  If .text has a "valid" symbol type, then it will be found (instead of, say, "_main"), and I'm not sure if the unwinding/stepping would work right.
How about:

    if (coff_symbol_type == 0)
        return lldb::eSymbolTypeInvalid;
    if (coff_symbol_type >> llvm::COFF::SCT_COMPLEX_TYPE_SHIFT) == llvm::COFF::IMAGE_SYM_DTYPE_FUNCTION)
        return lldb::eSymbolTypeCode;
    return lldb::eSymbolTypeData;

I feel like we can at least do a little better than always returning `eSymbolTypeInvalid` if it's not a function without too much work.


http://reviews.llvm.org/D16563