diff mbox

Check for POSIX_MADV_WILLNEED to support building against the LSB

Message ID 4FD57332.6020309@linaro.org
State New
Headers show

Commit Message

Michael Hope June 11, 2012, 4:25 a.m. UTC 
The Linux Standard Base APIs include posix_madvise() but don't define
values for the 'advice' argument.  Check to see if POSIX_MADV_WILLNEED
is defined before using.

OK for trunk?

-- Michael

2012-06-11  Michael Hope  <michael.hope@linaro.org>

	* dwarf2read.c (dwarf2_read_section): Check for
	POSIX_MADV_WILLNEED.

Comments

Mark Kettenis June 11, 2012, 5:59 a.m. UTC | #1 
> Date: Mon, 11 Jun 2012 16:25:22 +1200
> From: Michael Hope <michael.hope@linaro.org>
> 
> The Linux Standard Base APIs include posix_madvise() but don't define
> values for the 'advice' argument.  Check to see if POSIX_MADV_WILLNEED
> is defined before using.

What real-world problem does this fix?  It makes no sense to have
posix_madvise() but not define any values for the 'advice' argument.
I'd say this is a "bug" in LSB.  Either they should drop
posix_madvise() from the standard, or include at least the 'advice'
values required by POSIX.

> 2012-06-11  Michael Hope  <michael.hope@linaro.org>
> 
> 	* dwarf2read.c (dwarf2_read_section): Check for
> 	POSIX_MADV_WILLNEED.
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 589361e..91fab5a 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1773,7 +1773,7 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
> 
>         if ((caddr_t)info->buffer != MAP_FAILED)
>          {
> -#if HAVE_POSIX_MADVISE
> +#if HAVE_POSIX_MADVISE && defined(POSIX_MADV_WILLNEED)
>            posix_madvise (info->map_addr, info->map_len, POSIX_MADV_WILLNEED);
>   #endif
>            return;
>
Michael Hope June 11, 2012, 6:16 a.m. UTC | #2 
On 11 June 2012 17:59, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Mon, 11 Jun 2012 16:25:22 +1200
>> From: Michael Hope <michael.hope@linaro.org>
>>
>> The Linux Standard Base APIs include posix_madvise() but don't define
>> values for the 'advice' argument.  Check to see if POSIX_MADV_WILLNEED
>> is defined before using.
>
> What real-world problem does this fix?

Being able to build GDB using the LSB tools so that you can make a
binary release that works on any Linux distro from RHEL4 and above.
We use it for the linaro-toolchain-binaries.

> It makes no sense to have
> posix_madvise() but not define any values for the 'advice' argument.
> I'd say this is a "bug" in LSB.  Either they should drop
> posix_madvise() from the standard, or include at least the 'advice'
> values required by POSIX.

The LSB has a policy of not dropping functions.  I can suggest that
they add the advise values for a later version but that will take some
time and won't cover existing LSB releases.

I could change this to a configure test similar to HAS_MADVISE but
it's bad practice to separate the check from the use.

-- Michael

>> 2012-06-11  Michael Hope  <michael.hope@linaro.org>
>>
>>       * dwarf2read.c (dwarf2_read_section): Check for
>>       POSIX_MADV_WILLNEED.
>>
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index 589361e..91fab5a 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -1773,7 +1773,7 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
>>
>>         if ((caddr_t)info->buffer != MAP_FAILED)
>>          {
>> -#if HAVE_POSIX_MADVISE
>> +#if HAVE_POSIX_MADVISE && defined(POSIX_MADV_WILLNEED)
>>            posix_madvise (info->map_addr, info->map_len, POSIX_MADV_WILLNEED);
>>   #endif
>>            return;
>>
Mark Kettenis June 11, 2012, 8:18 a.m. UTC | #3 
> From: Michael Hope <michael.hope@linaro.org>
> Date: Mon, 11 Jun 2012 18:16:51 +1200
> 
> On 11 June 2012 17:59, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Date: Mon, 11 Jun 2012 16:25:22 +1200
> >> From: Michael Hope <michael.hope@linaro.org>
> >>
> >> The Linux Standard Base APIs include posix_madvise() but don't define
> >> values for the 'advice' argument.  Check to see if POSIX_MADV_WILLNEED
> >> is defined before using.
> >
> > What real-world problem does this fix?
> 
> Being able to build GDB using the LSB tools so that you can make a
> binary release that works on any Linux distro from RHEL4 and above.
> We use it for the linaro-toolchain-binaries.

I fear you're using a broken toolchain, because...

> > It makes no sense to have
> > posix_madvise() but not define any values for the 'advice' argument.
> > I'd say this is a "bug" in LSB.  Either they should drop
> > posix_madvise() from the standard, or include at least the 'advice'
> > values required by POSIX.
> 
> The LSB has a policy of not dropping functions.  I can suggest that
> they add the advise values for a later version but that will take some
> time and won't cover existing LSB releases.

Both posix_madvise():

http://www.linuxbase.org/navigator/browse/int_single.php?cmd=list-by-name&Iname=posix_madvise&Ilibrary=libc

and POSIX_MADV_WILLNEED:

http://www.linuxbase.org/navigator/browse/headgroup.php?cmd=list-byheadgroup&HGid=56

were added in LSB 3.2.
Michael Hope June 11, 2012, 9:39 p.m. UTC | #4 
On 11 June 2012 20:18, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Michael Hope <michael.hope@linaro.org>
>> Date: Mon, 11 Jun 2012 18:16:51 +1200
>>
>> On 11 June 2012 17:59, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> Date: Mon, 11 Jun 2012 16:25:22 +1200
>> >> From: Michael Hope <michael.hope@linaro.org>
>> >>
>> >> The Linux Standard Base APIs include posix_madvise() but don't define
>> >> values for the 'advice' argument.  Check to see if POSIX_MADV_WILLNEED
>> >> is defined before using.
>> >
>> > What real-world problem does this fix?
>>
>> Being able to build GDB using the LSB tools so that you can make a
>> binary release that works on any Linux distro from RHEL4 and above.
>> We use it for the linaro-toolchain-binaries.
>
> I fear you're using a broken toolchain, because...
>
>> > It makes no sense to have
>> > posix_madvise() but not define any values for the 'advice' argument.
>> > I'd say this is a "bug" in LSB.  Either they should drop
>> > posix_madvise() from the standard, or include at least the 'advice'
>> > values required by POSIX.
>>
>> The LSB has a policy of not dropping functions.  I can suggest that
>> they add the advise values for a later version but that will take some
>> time and won't cover existing LSB releases.
>
> Both posix_madvise():
>
> http://www.linuxbase.org/navigator/browse/int_single.php?cmd=list-by-name&Iname=posix_madvise&Ilibrary=libc
>
> and POSIX_MADV_WILLNEED:
>
> http://www.linuxbase.org/navigator/browse/headgroup.php?cmd=list-byheadgroup&HGid=56
>
> were added in LSB 3.2.

I've dug a bit deeper and there seems to be a bug in the LSB 3.2
release.  posix_madvise() was defined but the advice constants weren't
added to the header files.  See sys/mman.h in:
 http://ftp.linuxbase.org/pub/lsb/lsbdev/released-3.2.0/binary/amd64/lsb-build-base-3.2.2-1.x86_64.rpm

Ubuntu ships with this version.  I'll change our build system to use
4.1.3 in the 3.2 compatibility mode instead.

Ta,

-- Michael
diff mbox

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 589361e..91fab5a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1773,7 +1773,7 @@  dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)

        if ((caddr_t)info->buffer != MAP_FAILED)
         {
-#if HAVE_POSIX_MADVISE
+#if HAVE_POSIX_MADVISE && defined(POSIX_MADV_WILLNEED)
           posix_madvise (info->map_addr, info->map_len, POSIX_MADV_WILLNEED);
  #endif
           return;