RFR: 8033251: Use DWARF debug symbols for Linux 32-bit as default

[Coleen Phillimore]

I thought you'd be able to get rid of the entire DEBUG_BINARIES setting 
with the cleanup too.   Or is that additional cleanup?

Coleen

On 4/1/14 2:38 AM, David Holmes wrote:
> Hi Erik,
>
> On 31/03/2014 10:28 PM, Erik Helin wrote:
>> Hi David,
>>
>> thanks for having a look! Please see my reply inline.
>
> Thanks for the additional cleanup!
>
> Looks good.
>
> David
>
>
>> On 2014-03-29 05:43, David Holmes wrote:
>>> On 29/03/2014 4:26 AM, Coleen Phillimore wrote:
>>>>
>>>> On 3/28/14 12:44 PM, Erik Helin wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> thanks for having a look at the patch!
>>>>>
>>>>> On 2014-03-28 17:30, Coleen Phillimore wrote:
>>>>>>
>>>>>> I thought you'd be able to remove the DEBUG_BINARIES logic with this
>>>>>> change.  It appears there are no platforms left that use stabs with
>>>>>> gcc.
>>>>>
>>>>> Yeah, I also noticed that this change enables cleanups in the
>>>>> Makefiles. However, I think it is better to do these cleanups as a
>>>>> second change after this one, what do you think?
>>>>
>>>> A cleanup CR is fine.
>>>
>>> Unless the cleanup is coming "tomorrow" I disagree. What have now is
>>> comments that make no sense given the change that has been made. And as
>>> every defined platform seems to use -g we can get rid of all this
>>> per-platform logic and simply check for clang. All of which can be done
>>> without doing the DEBUG_BINARIES cleanup. Eg delete the per-platform
>>> flags and simply have, for example,
>>>
>>>      # Override per-platform if the default -g doesn't suite
>>>      OPT_CFLAGS += $(OPT_CFLAGS/$(BUILDARCH))
>>>      ifeq ($(OPT_CFLAGS/$(BUILDARCH)),)
>>>        ifeq ($(USE_CLANG), true)
>>>          # Clang doesn't understand -gstabs
>>>          OPT_CFLAGS += -g
>>>        else
>>>          OPT_CFLAGS += -gstabs
>>>        endif
>>>      endif
>>
>> I wanted this change to only be about changing from STABS to DWARF, but
>> based on your feedback I cleaned up some code in the makefile as well:
>>
>> - Incremental webrev:
>>    http://cr.openjdk.java.net/~ehelin/8033251/webrev.00-01/
>> - New webrev:
>>    http://cr.openjdk.java.net/~ehelin/8033251/webrev.01/
>>
>> The default is now to use DWARF for all platforms, but this can be
>> changed by using CFLAGS, for example FASTDEBUG_CFLAGS/amd64=-gstabs will
>> give you STABS debug symbols for a fastdebug build on 64-bit Linux.
>> Having -g as default also enables us to get rid of all the $(USE_CLANG)
>> checks.
>>
>> The new patch also fixes a small bug where FASTDEBUG_CFLAGS was set to
>> $(DEBUG_CFLAGS/$(BUILDARCH)), it should be set to
>> $(FASTDEBUG_CFLAGS/$(BUILDARCH)).
>>
>> Testing of new patch:
>> - JPRT
>> - Building builds for different targets with different debug symbols
>>    locally.
>>
>> Thanks,
>> Erik
>>
>>> In the future this flag should also potentially be coming in via
>>> spec.gmk.
>>>
>>> David
>>>
>>>>>
>>>>> On 2014-03-28 17:30, Coleen Phillimore wrote:
>>>>>> Does this change add -g for saproc, adlc and jsig compilations also?
>>>>> It seems like saproc uses SA_DEBUG_CFLAGS which only is being set if
>>>>> DEBUG_BINARIES=true. If DEBUG_BINARIES is not true, then it seems 
>>>>> like
>>>>> we don't generate debug symbols (please correct me if I'm wrong).
>>>>>
>>>>> The above also seems to be the case for libjsig.so.
>>>>>
>>>>
>>>> These seem broken, but I don't know the makefiles well enough to
>>>> comment.
>>>>
>>>> I think this change is good.  People have been asking for it for years
>>>> and now that the size problem with dwarf has been fixed, I'm glad you
>>>> are making this change.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>> Looking in make/linux/makefiles/adlc.make it seems like adlc is using
>>>>> -g for GCC 3.3 or newer.
>>>>>
>>>>> Thanks,
>>>>> Erik
>>>>>
>>>>>> Coleen
>>>>>>
>>>>>> On 3/28/14 12:12 PM, Erik Helin wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> this patch changes the Linux Makefiles to use DWARF as the default
>>>>>>> debug symbol format for Linux 32-bit instead of STABS. The main
>>>>>>> arguments for using DWARF are:
>>>>>>>
>>>>>>>   - DWARF provides a superior debugging experience compared to 
>>>>>>> STABS.
>>>>>>>   - STABS support in GCC/GDB is in maintenance mode [0].
>>>>>>>   - External tools (e.g. perf on Linux) often understands DWARF but
>>>>>>> not
>>>>>>>     STABS.
>>>>>>>   - Compilation on 32-bit Linux can make use of precompiled 
>>>>>>> headers.
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~ehelin/8033251/webrev.00/
>>>>>>>
>>>>>>> Testing:
>>>>>>> - Compiling with GCC 4.3.4, GCC 4.7.3 and GCC 4.8.1
>>>>>>> - JPRT
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Erik
>>>>>>>
>>>>>>> [0]: https://sourceware.org/ml/binutils/2013-01/msg00028.html
>>>>>>
>>>>