diff mbox

[libdrm,01/11] symbols-check: add new meta-script

Message ID 20180404154145.27607-1-eric.engestrom@imgtec.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Engestrom April 4, 2018, 3:41 p.m. UTC 
Note: copied from Mesa

Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
 meson.build   |  1 +
 symbols-check | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100755 symbols-check

Comments

Eric Engestrom April 4, 2018, 4:28 p.m. UTC | #1 
On Wednesday, 2018-04-04 16:41:35 +0100, Eric Engestrom wrote:
> Note: copied from Mesa
> 
> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---
>  meson.build   |  1 +
>  symbols-check | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 80 insertions(+)
>  create mode 100755 symbols-check
> 
> diff --git a/meson.build b/meson.build
> index a725f05d342bbec4df18..c035a00c6747b8d46a9b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -272,6 +272,7 @@ pkg.generate(
>  
>  env_test = environment()
>  env_test.set('NM', find_program('nm').path())
> +env_test.set('top_srcdir', meson.source_root())
>  
>  if with_libkms
>    subdir('libkms')
> diff --git a/symbols-check b/symbols-check
> new file mode 100755
> index 00000000000000000000..bac466d93dcb45cee0bb
> --- /dev/null
> +++ b/symbols-check
> @@ -0,0 +1,79 @@
> +#!/bin/sh
> +set -eu
> +set -o pipefail
> +
> +# Platform specific symbols
> +# These will simply be ignored
> +PLAT_FUNCS="
> +__bss_start
> +_init
> +_fini
> +_end
> +_edata
> +
> +# From tegra-symbol-check
> +__bss_end__
> +__bss_start__
> +__bss_start
> +__end__
> +_bss_end__
> +_edata
> +_end
> +_fini
> +_init

Haha, oops... I had noticed that one of the old scripts had more platform
symbols than the rest, and I wanted to check if/when those were needed
so I just stuffed them here in the mean time, but then I forgot :]

I'll check this when I have the time (not this week), or if anyone knows..?

In the mean time, please review the rest and ignore these 10 lines :)

> +"
> +
> +if [ -z "$LIB" ]; then
> +  echo "\$LIB needs to be defined for autotools to be able to run this test"
> +  exit 1
> +fi
> +
> +# The lib name is passed in with Meson but autotools doesn't support that
> +# so it needs to be hardcoded and overwritten here
> +if [ $# -ge 1 ]; then
> +  LIB=$1
> +fi
> +
> +if ! [ -f "$LIB" ]; then
> +  echo "lib $LIB doesn't exist"
> +  exit 1
> +fi
> +
> +if [ -z "$NM" ]; then
> +  echo "\$NM is undefined or empty"
> +  exit 1
> +elif ! command -v $NM >/dev/null; then
> +  echo "\$NM is not a valid command"
> +  exit 1
> +fi
> +
> +AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
> +
> +NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
> +  echo "$REQ_FUNCS" | grep -q "^$func$" && continue
> +  echo "$PLAT_FUNCS" | grep -q "^$func$" && continue
> +
> +  echo $func
> +done)
> +
> +REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
> +  echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
> +
> +  echo $func
> +done)
> +
> +if [ -n "$NEW_ABI" ]; then
> +  echo "New ABI detected - If intentional, update the test."
> +  echo "$NEW_ABI"
> +fi
> +
> +if [ -n "$REMOVED_ABI" ]; then
> +  echo "ABI break detected - Required symbol(s) no longer exported!"
> +  echo "$REMOVED_ABI"
> +fi
> +
> +if [ -z "$NEW_ABI" ] && [ -z "$REMOVED_ABI" ]; then
> +  exit 0
> +else
> +  exit 1
> +fi
> -- 
> Cheers,
>   Eric
>
Emil Velikov April 4, 2018, 5:39 p.m. UTC | #2 
On 4 April 2018 at 17:28, Eric Engestrom <eric.engestrom@imgtec.com> wrote:
> On Wednesday, 2018-04-04 16:41:35 +0100, Eric Engestrom wrote:
>> Note: copied from Mesa
>>
>> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
>> ---
>>  meson.build   |  1 +
>>  symbols-check | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 80 insertions(+)
>>  create mode 100755 symbols-check
>>
>> diff --git a/meson.build b/meson.build
>> index a725f05d342bbec4df18..c035a00c6747b8d46a9b 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -272,6 +272,7 @@ pkg.generate(
>>
>>  env_test = environment()
>>  env_test.set('NM', find_program('nm').path())
>> +env_test.set('top_srcdir', meson.source_root())
>>
>>  if with_libkms
>>    subdir('libkms')
>> diff --git a/symbols-check b/symbols-check
>> new file mode 100755
>> index 00000000000000000000..bac466d93dcb45cee0bb
>> --- /dev/null
>> +++ b/symbols-check
>> @@ -0,0 +1,79 @@
>> +#!/bin/sh
>> +set -eu
>> +set -o pipefail
>> +
>> +# Platform specific symbols
>> +# These will simply be ignored
>> +PLAT_FUNCS="
>> +__bss_start
>> +_init
>> +_fini
>> +_end
>> +_edata
>> +
>> +# From tegra-symbol-check
>> +__bss_end__
>> +__bss_start__
>> +__bss_start
>> +__end__
>> +_bss_end__
>> +_edata
>> +_end
>> +_fini
>> +_init
>
> Haha, oops... I had noticed that one of the old scripts had more platform
> symbols than the rest, and I wanted to check if/when those were needed
> so I just stuffed them here in the mean time, but then I forgot :]
>
> I'll check this when I have the time (not this week), or if anyone knows..?
>
> In the mean time, please review the rest and ignore these 10 lines :)
>
The extra __ suffix/prefixed ones are ARM specific. They seems to be
introduced from day 1 in glibc, without any obvious reason.
They're just aliases - __end__, _bss_end__, etc are identical to _end

If you want to cater for GNU/Hurd - the following three should be
listed as well.

_fbss
_fdata
_ftext

I haven't had time to read through the patches, although +1 on the overall idea.
Will try to get to it sometime tomorrow.

-Emil
Eric Engestrom April 5, 2018, 9:36 a.m. UTC | #3 
On Wednesday, 2018-04-04 18:39:48 +0100, Emil Velikov wrote:
> On 4 April 2018 at 17:28, Eric Engestrom <eric.engestrom@imgtec.com> wrote:
> > On Wednesday, 2018-04-04 16:41:35 +0100, Eric Engestrom wrote:
> >> Note: copied from Mesa
> >>
> >> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> >> ---
> >>  meson.build   |  1 +
> >>  symbols-check | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 80 insertions(+)
> >>  create mode 100755 symbols-check
> >>
> >> diff --git a/meson.build b/meson.build
> >> index a725f05d342bbec4df18..c035a00c6747b8d46a9b 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -272,6 +272,7 @@ pkg.generate(
> >>
> >>  env_test = environment()
> >>  env_test.set('NM', find_program('nm').path())
> >> +env_test.set('top_srcdir', meson.source_root())
> >>
> >>  if with_libkms
> >>    subdir('libkms')
> >> diff --git a/symbols-check b/symbols-check
> >> new file mode 100755
> >> index 00000000000000000000..bac466d93dcb45cee0bb
> >> --- /dev/null
> >> +++ b/symbols-check
> >> @@ -0,0 +1,79 @@
> >> +#!/bin/sh
> >> +set -eu
> >> +set -o pipefail
> >> +
> >> +# Platform specific symbols
> >> +# These will simply be ignored
> >> +PLAT_FUNCS="
> >> +__bss_start
> >> +_init
> >> +_fini
> >> +_end
> >> +_edata
> >> +
> >> +# From tegra-symbol-check
> >> +__bss_end__
> >> +__bss_start__
> >> +__bss_start
> >> +__end__
> >> +_bss_end__
> >> +_edata
> >> +_end
> >> +_fini
> >> +_init
> >
> > Haha, oops... I had noticed that one of the old scripts had more platform
> > symbols than the rest, and I wanted to check if/when those were needed
> > so I just stuffed them here in the mean time, but then I forgot :]
> >
> > I'll check this when I have the time (not this week), or if anyone knows..?
> >
> > In the mean time, please review the rest and ignore these 10 lines :)
> >
> The extra __ suffix/prefixed ones are ARM specific. They seems to be
> introduced from day 1 in glibc, without any obvious reason.
> They're just aliases - __end__, _bss_end__, etc are identical to _end
> 
> If you want to cater for GNU/Hurd - the following three should be
> listed as well.
> 
> _fbss
> _fdata
> _ftext

Thanks for the explanation!
I think I'll just add all these to the platform-specific list on both
mesa and libdrm, they have specific enough names (beginning with
underscore, for starters) that no entrypoint of ours should ever match.

> 
> I haven't had time to read through the patches, although +1 on the overall idea.
> Will try to get to it sometime tomorrow.

Thanks, but I won't have time to push anything until next week anyway,
so there's no rush ^^

> 
> -Emil
Emil Velikov Sept. 6, 2018, 3:49 p.m. UTC | #4 
On 4 April 2018 at 16:41, Eric Engestrom <eric.engestrom@imgtec.com> wrote:

> --- /dev/null
> +++ b/symbols-check
> @@ -0,0 +1,79 @@
> +#!/bin/sh
> +set -eu
> +set -o pipefail
> +
We could drop the execute bit, shebang and set. Pretty much all of it
is handled by the callers (modulo pipefail)

> +if [ -z "$LIB" ]; then
> +  echo "\$LIB needs to be defined for autotools to be able to run this test"
> +  exit 1
> +fi
> +
> +# The lib name is passed in with Meson but autotools doesn't support that
> +# so it needs to be hardcoded and overwritten here
> +if [ $# -ge 1 ]; then
> +  LIB=$1
> +fi
> +
> +if ! [ -f "$LIB" ]; then
> +  echo "lib $LIB doesn't exist"
> +  exit 1
> +fi
> +
> +if [ -z "$NM" ]; then
> +  echo "\$NM is undefined or empty"
> +  exit 1

I would drop all these - set -u will provide reasonable error handling

HTH
Emil
diff mbox

Patch

diff --git a/meson.build b/meson.build
index a725f05d342bbec4df18..c035a00c6747b8d46a9b 100644
--- a/meson.build
+++ b/meson.build
@@ -272,6 +272,7 @@  pkg.generate(
 
 env_test = environment()
 env_test.set('NM', find_program('nm').path())
+env_test.set('top_srcdir', meson.source_root())
 
 if with_libkms
   subdir('libkms')
diff --git a/symbols-check b/symbols-check
new file mode 100755
index 00000000000000000000..bac466d93dcb45cee0bb
--- /dev/null
+++ b/symbols-check
@@ -0,0 +1,79 @@ 
+#!/bin/sh
+set -eu
+set -o pipefail
+
+# Platform specific symbols
+# These will simply be ignored
+PLAT_FUNCS="
+__bss_start
+_init
+_fini
+_end
+_edata
+
+# From tegra-symbol-check
+__bss_end__
+__bss_start__
+__bss_start
+__end__
+_bss_end__
+_edata
+_end
+_fini
+_init
+"
+
+if [ -z "$LIB" ]; then
+  echo "\$LIB needs to be defined for autotools to be able to run this test"
+  exit 1
+fi
+
+# The lib name is passed in with Meson but autotools doesn't support that
+# so it needs to be hardcoded and overwritten here
+if [ $# -ge 1 ]; then
+  LIB=$1
+fi
+
+if ! [ -f "$LIB" ]; then
+  echo "lib $LIB doesn't exist"
+  exit 1
+fi
+
+if [ -z "$NM" ]; then
+  echo "\$NM is undefined or empty"
+  exit 1
+elif ! command -v $NM >/dev/null; then
+  echo "\$NM is not a valid command"
+  exit 1
+fi
+
+AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
+
+NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
+  echo "$REQ_FUNCS" | grep -q "^$func$" && continue
+  echo "$PLAT_FUNCS" | grep -q "^$func$" && continue
+
+  echo $func
+done)
+
+REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
+  echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
+
+  echo $func
+done)
+
+if [ -n "$NEW_ABI" ]; then
+  echo "New ABI detected - If intentional, update the test."
+  echo "$NEW_ABI"
+fi
+
+if [ -n "$REMOVED_ABI" ]; then
+  echo "ABI break detected - Required symbol(s) no longer exported!"
+  echo "$REMOVED_ABI"
+fi
+
+if [ -z "$NEW_ABI" ] && [ -z "$REMOVED_ABI" ]; then
+  exit 0
+else
+  exit 1
+fi