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

Tue Apr 1 14:03:27 UTC 2014

On 2014-04-01 15:28, Coleen Phillimore wrote:
>
> On 4/1/14 9:03 AM, Erik Helin wrote:
>> 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?
>
> Yes, I just misunderstood David's cleanup request.

Ok, thanks!

> Can you file an RFE to remove DEBUG_BINARIES (if you haven't already)?

Sure, https://bugs.openjdk.java.net/browse/JDK-8038957.

Thanks,
Erik

> thanks,
> Coleen
>
>>
>> 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
>>>>>>>>>
>>>>>>>
>>>
>