[Lldb-commits] [review request] move cxa_demangle.cpp to another directory to avoid gcc/rtti errors

[Malea, Daniel]

I found a slight bug in the patch I posted yesterday; turns out that checking CXX against g++ breaks when the user specifies the full path to their gcc installation.

Here's the updated version that uses a less fragile check; do you think it's OK to commit Greg?


Thanks,
Dan


On 2012-12-06, at 8:15 PM, Malea, Daniel wrote:

Hey Greg, thanks for looking into this! After some discussions with Matt, I think I found Option 4:

Enable -frtti for all of Core, but *only* if GCC is being used. This way, the clang builds on Linux/Mac will continue to work as they do now, but if someone is using GCC, the build will not be broken. This approach also has the benefit that we won't have to regenerate the xcode project/workspace checked into svn.

I know there might be some concerns with bloating the size of the binaries by using -frtti, but I noticed a marginal difference in binary sizes: liblldb.so weighs in at 535MB (clang with -fno-rtti) vs 531MB (gcc with -frtti) for a Debug build on my Linux machine.

The attached patch implements the above with minimal Makefile hackery. The only surprising thing in it was that I had to also enable -frtti for lldbUtility, and also link in the clangRewrite static libs so that some vtable references can be resolved correctly. Naturally all of this is only enabled if g++ is the compiler.


Cheerio,
Dan




On 2012-12-06, at 1:53 PM, Greg Clayton wrote:

> I read the actual code "dynamic_cast<__void*>" in my mind as "dynamic_cast<void*>" (no double underscore, just the type "void*"), and thus the reason for my bad patch... I am still waking up.
>
> On Dec 6, 2012, at 10:50 AM, Greg Clayton <gclayton at apple.com<mailto:gclayton at apple.com>> wrote:
>
>> Don't use this patch. After speaking with Howard Hinnant, the dynamic_cast is indeed needed.
>>
>> On Dec 6, 2012, at 10:14 AM, Greg Clayton <gclayton at apple.com<mailto:gclayton at apple.com>> wrote:
>>
>>> Option #3: there are only two dynamic_casts and both of them can be static_casts.
>>>
>>> <static.patch>
>>>
>>> Try this out.
>>>
>>> On Dec 5, 2012, at 3:13 PM, "Malea, Daniel" <daniel.malea at intel.com<mailto:daniel.malea at intel.com>> wrote:
>>>
>>>> Hi all,
>>>>
>>>> As I'm trying to fix the gcc build, I ran into some trouble with source/Core/cxa_demangle.cpp: as it uses dynamic_cast<>, gcc complains if it is compiled with -fno-rtti.
>>>>
>>>> I'm afraid I couldn't find any (non-hacky) way to change the compile flags for just that one file. I see two ways to fix the problem "correctly":
>>>>
>>>> 1. Define REQUIRES_RTTI = 1 in source/Core/Makefile, thereby compiling many more files with -rtti than actually need it. It fixes the build, but is not ideal.
>>>>
>>>> 2. Move cxa_demangle.cpp to another directory (I propose "source/Demangle" thereby creating a new static library "liblldbDemangle.a") and set the required RTTI options for just that one directory.
>>>>
>>>> What does the mailing list think? The attached patch implements option 2 above, and also fixes lib/Makefile to correctly include libclangRewriteCore.a and libclangRewriteFrontend.a as those are apparently required...
>>>>
>>>>
>>>> Cheers,
>>>> Dan
>>>>
>>>> <move_cxa_demangle.patch>_______________________________________________
>>>> lldb-commits mailing list
>>>> lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu<mailto:lldb-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>

<enable_rtti_as_needed_on_gcc.patch>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: enable_rtti_on_gcc2.patch
Type: application/octet-stream
Size: 2054 bytes
Desc: enable_rtti_on_gcc2.patch
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20121207/315f8f88/attachment.obj>