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

Tue Apr 1 13:28:10 UTC 2014

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.  Can you file an RFE 
to remove DEBUG_BINARIES (if you haven't already)?
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
>>>>>>>>
>>>>>>
>>