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

Tue Apr 1 13:03:08 UTC 2014

On 2014-04-01 13:38, Coleen Phillimore wrote:
>
> I thought you'd be able to get rid of the entire DEBUG_BINARIES setting
> with the cleanup too.   Or is that additional cleanup?

I view getting rid of DEBUG_BINARIES as an additional cleanup since 
DEBUG_BINARIES are used for platforms other than Linux and this patch 
only changes the Linux makefiles.

Are you ok with this?

Thanks,
Erik

> 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
>>>>>>>
>>>>>
>