Document and fix --enable-bind-now [BZ #21015]

Message ID 07bf9558-2d72-d0da-62bb-c39da2ca6814@redhat.com
State Not applicable
Headers

Commit Message

Florian Weimer Jan. 4, 2017, 5:52 p.m. UTC
  On 01/02/2017 07:46 PM, Florian Weimer wrote:
> 2017-01-02  Florian Weimer  <fweimer@redhat.com>
>
> 	[BZ #21015]
> 	* manual/install.texi (Configuring and compiling): Document
> 	--enable-bind-now.
> 	* Makeconfig [bind-now] (LDFLAGS-lib.so): Set.
> 	(build-shlib-helper): Use $(LDFLAGS-lib.so).
> 	(format.lds): Likewise.
> 	[bind-now] (LDFLAGS-c.so): Remove.
> 	* sysdeps/x86_64/localplt.data (libm.so): matherr relocation can
> 	be R_X86_64_GLOB_DAT.
> 	* sysdeps/unix/sysv/linux/i386/localplt.data (libm.so): matherr
> 	relocation can be R_386_GLOB_DAT.
> 	* sysdeps/unix/sysv/linux/alpha/localplt.data (libm.so): matherr
> 	relocaiton can be R_ALPHA_GLOB_DAT.

The patch is incomplete because it does not cover gconv modules, which 
are compiled as, well, modules (just like the test DSOs in elf/).

This patch on top fixes that omission (hopefully it's not garbled too much):

  ifeq (yes,$(build-shared))

But I wonder if it's the right approach.  I don't think it's worth 
adding another kind of module just for this feature.

Thanks,
Florian
  

Comments

Florian Weimer Feb. 28, 2017, 3:04 p.m. UTC | #1
On 01/04/2017 06:52 PM, Florian Weimer wrote:
> On 01/02/2017 07:46 PM, Florian Weimer wrote:
>> 2017-01-02  Florian Weimer  <fweimer@redhat.com>
>>
>>     [BZ #21015]
>>     * manual/install.texi (Configuring and compiling): Document
>>     --enable-bind-now.
>>     * Makeconfig [bind-now] (LDFLAGS-lib.so): Set.
>>     (build-shlib-helper): Use $(LDFLAGS-lib.so).
>>     (format.lds): Likewise.
>>     [bind-now] (LDFLAGS-c.so): Remove.
>>     * sysdeps/x86_64/localplt.data (libm.so): matherr relocation can
>>     be R_X86_64_GLOB_DAT.
>>     * sysdeps/unix/sysv/linux/i386/localplt.data (libm.so): matherr
>>     relocation can be R_386_GLOB_DAT.
>>     * sysdeps/unix/sysv/linux/alpha/localplt.data (libm.so): matherr
>>     relocaiton can be R_ALPHA_GLOB_DAT.
>
> The patch is incomplete because it does not cover gconv modules, which
> are compiled as, well, modules (just like the test DSOs in elf/).
>
> This patch on top fixes that omission (hopefully it's not garbled too
> much):
>
> diff --git a/iconvdata/Makefile b/iconvdata/Makefile
> index 04157b2..eb744da 100644
> --- a/iconvdata/Makefile
> +++ b/iconvdata/Makefile
> @@ -63,6 +63,10 @@ modules      := ISO8859-1 ISO8859-2 ISO8859-3
> ISO8859-4 ISO8859-5             \
>            MAC-CENTRALEUROPE KOI8-RU ISO8859-9E                          \
>            CP770 CP771 CP772 CP773 CP774
>
> +ifeq ($(bind-now),yes)
> +LDFLAGS.so += -Wl,-z,now
> +endif
> +
>  modules.so := $(addsuffix .so, $(modules))
>
>  ifeq (yes,$(build-shared))
>
> But I wonder if it's the right approach.  I don't think it's worth
> adding another kind of module just for this feature.

Ping?

Original patch is here:

  https://sourceware.org/ml/libc-alpha/2017-01/msg00060.html

Thanks,
Florian
  
Carlos O'Donell March 1, 2017, 4:47 p.m. UTC | #2
On 02/28/2017 10:04 AM, Florian Weimer wrote:
> On 01/04/2017 06:52 PM, Florian Weimer wrote:
>> On 01/02/2017 07:46 PM, Florian Weimer wrote:
>>> 2017-01-02  Florian Weimer  <fweimer@redhat.com>
>>>
>>>     [BZ #21015]
>>>     * manual/install.texi (Configuring and compiling): Document
>>>     --enable-bind-now.
>>>     * Makeconfig [bind-now] (LDFLAGS-lib.so): Set.
>>>     (build-shlib-helper): Use $(LDFLAGS-lib.so).
>>>     (format.lds): Likewise.
>>>     [bind-now] (LDFLAGS-c.so): Remove.
>>>     * sysdeps/x86_64/localplt.data (libm.so): matherr relocation can
>>>     be R_X86_64_GLOB_DAT.
>>>     * sysdeps/unix/sysv/linux/i386/localplt.data (libm.so): matherr
>>>     relocation can be R_386_GLOB_DAT.
>>>     * sysdeps/unix/sysv/linux/alpha/localplt.data (libm.so): matherr
>>>     relocaiton can be R_ALPHA_GLOB_DAT.
>>
>> The patch is incomplete because it does not cover gconv modules, which
>> are compiled as, well, modules (just like the test DSOs in elf/).
>>
>> This patch on top fixes that omission (hopefully it's not garbled too
>> much):
>>
>> diff --git a/iconvdata/Makefile b/iconvdata/Makefile
>> index 04157b2..eb744da 100644
>> --- a/iconvdata/Makefile
>> +++ b/iconvdata/Makefile
>> @@ -63,6 +63,10 @@ modules      := ISO8859-1 ISO8859-2 ISO8859-3
>> ISO8859-4 ISO8859-5             \
>>            MAC-CENTRALEUROPE KOI8-RU ISO8859-9E                          \
>>            CP770 CP771 CP772 CP773 CP774
>>
>> +ifeq ($(bind-now),yes)
>> +LDFLAGS.so += -Wl,-z,now
>> +endif
>> +
>>  modules.so := $(addsuffix .so, $(modules))
>>
>>  ifeq (yes,$(build-shared))
>>
>> But I wonder if it's the right approach.  I don't think it's worth
>> adding another kind of module just for this feature.

I don't follow your question here, could you expand what you mean by
"another kind of module?"

> Ping?
> 
> Original patch is here:
> 
>  https://sourceware.org/ml/libc-alpha/2017-01/msg00060.html

Your original patch looks good to me.
  
Florian Weimer March 1, 2017, 4:52 p.m. UTC | #3
On 03/01/2017 05:47 PM, Carlos O'Donell wrote:
> On 02/28/2017 10:04 AM, Florian Weimer wrote:
>> On 01/04/2017 06:52 PM, Florian Weimer wrote:
>>> On 01/02/2017 07:46 PM, Florian Weimer wrote:
>>>> 2017-01-02  Florian Weimer  <fweimer@redhat.com>
>>>>
>>>>     [BZ #21015]
>>>>     * manual/install.texi (Configuring and compiling): Document
>>>>     --enable-bind-now.
>>>>     * Makeconfig [bind-now] (LDFLAGS-lib.so): Set.
>>>>     (build-shlib-helper): Use $(LDFLAGS-lib.so).
>>>>     (format.lds): Likewise.
>>>>     [bind-now] (LDFLAGS-c.so): Remove.
>>>>     * sysdeps/x86_64/localplt.data (libm.so): matherr relocation can
>>>>     be R_X86_64_GLOB_DAT.
>>>>     * sysdeps/unix/sysv/linux/i386/localplt.data (libm.so): matherr
>>>>     relocation can be R_386_GLOB_DAT.
>>>>     * sysdeps/unix/sysv/linux/alpha/localplt.data (libm.so): matherr
>>>>     relocaiton can be R_ALPHA_GLOB_DAT.
>>>
>>> The patch is incomplete because it does not cover gconv modules, which
>>> are compiled as, well, modules (just like the test DSOs in elf/).
>>>
>>> This patch on top fixes that omission (hopefully it's not garbled too
>>> much):
>>>
>>> diff --git a/iconvdata/Makefile b/iconvdata/Makefile
>>> index 04157b2..eb744da 100644
>>> --- a/iconvdata/Makefile
>>> +++ b/iconvdata/Makefile
>>> @@ -63,6 +63,10 @@ modules      := ISO8859-1 ISO8859-2 ISO8859-3
>>> ISO8859-4 ISO8859-5             \
>>>            MAC-CENTRALEUROPE KOI8-RU ISO8859-9E                          \
>>>            CP770 CP771 CP772 CP773 CP774
>>>
>>> +ifeq ($(bind-now),yes)
>>> +LDFLAGS.so += -Wl,-z,now
>>> +endif
>>> +
>>>  modules.so := $(addsuffix .so, $(modules))
>>>
>>>  ifeq (yes,$(build-shared))
>>>
>>> But I wonder if it's the right approach.  I don't think it's worth
>>> adding another kind of module just for this feature.
>
> I don't follow your question here, could you expand what you mean by
> "another kind of module?"

Another way of compiling a shared object (“module”).  Overriding 
LDFLAGS.so this way avoids defining another category which is neither an 
installed library, nor a DSO used by tests.

Thanks,
Florian
  
Carlos O'Donell March 1, 2017, 6:58 p.m. UTC | #4
On 03/01/2017 11:52 AM, Florian Weimer wrote:
> On 03/01/2017 05:47 PM, Carlos O'Donell wrote:
>> On 02/28/2017 10:04 AM, Florian Weimer wrote:
>>> On 01/04/2017 06:52 PM, Florian Weimer wrote:
>>>> On 01/02/2017 07:46 PM, Florian Weimer wrote:
>>>>> 2017-01-02  Florian Weimer  <fweimer@redhat.com>
>>>>>
>>>>>     [BZ #21015]
>>>>>     * manual/install.texi (Configuring and compiling): Document
>>>>>     --enable-bind-now.
>>>>>     * Makeconfig [bind-now] (LDFLAGS-lib.so): Set.
>>>>>     (build-shlib-helper): Use $(LDFLAGS-lib.so).
>>>>>     (format.lds): Likewise.
>>>>>     [bind-now] (LDFLAGS-c.so): Remove.
>>>>>     * sysdeps/x86_64/localplt.data (libm.so): matherr relocation can
>>>>>     be R_X86_64_GLOB_DAT.
>>>>>     * sysdeps/unix/sysv/linux/i386/localplt.data (libm.so): matherr
>>>>>     relocation can be R_386_GLOB_DAT.
>>>>>     * sysdeps/unix/sysv/linux/alpha/localplt.data (libm.so): matherr
>>>>>     relocaiton can be R_ALPHA_GLOB_DAT.
>>>>
>>>> The patch is incomplete because it does not cover gconv modules, which
>>>> are compiled as, well, modules (just like the test DSOs in elf/).
>>>>
>>>> This patch on top fixes that omission (hopefully it's not garbled too
>>>> much):
>>>>
>>>> diff --git a/iconvdata/Makefile b/iconvdata/Makefile
>>>> index 04157b2..eb744da 100644
>>>> --- a/iconvdata/Makefile
>>>> +++ b/iconvdata/Makefile
>>>> @@ -63,6 +63,10 @@ modules      := ISO8859-1 ISO8859-2 ISO8859-3
>>>> ISO8859-4 ISO8859-5             \
>>>>            MAC-CENTRALEUROPE KOI8-RU ISO8859-9E                          \
>>>>            CP770 CP771 CP772 CP773 CP774
>>>>
>>>> +ifeq ($(bind-now),yes)
>>>> +LDFLAGS.so += -Wl,-z,now
>>>> +endif
>>>> +
>>>>  modules.so := $(addsuffix .so, $(modules))
>>>>
>>>>  ifeq (yes,$(build-shared))
>>>>
>>>> But I wonder if it's the right approach.  I don't think it's worth
>>>> adding another kind of module just for this feature.
>>
>> I don't follow your question here, could you expand what you mean by
>> "another kind of module?"
> 
> Another way of compiling a shared object (“module”). Overriding
> LDFLAGS.so this way avoids defining another category which is neither
> an installed library, nor a DSO used by tests.

Thanks, I understand now. Yes, your approach is practical. We can revisit
it in the future without much problem.
  

Patch

diff --git a/iconvdata/Makefile b/iconvdata/Makefile
index 04157b2..eb744da 100644
--- a/iconvdata/Makefile
+++ b/iconvdata/Makefile
@@ -63,6 +63,10 @@  modules      := ISO8859-1 ISO8859-2 ISO8859-3 
ISO8859-4 ISO8859-5             \
            MAC-CENTRALEUROPE KOI8-RU ISO8859-9E                          \
            CP770 CP771 CP772 CP773 CP774

+ifeq ($(bind-now),yes)
+LDFLAGS.so += -Wl,-z,now
+endif
+
  modules.so := $(addsuffix .so, $(modules))