Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.93k stars 551 forks source link

h2ph should emit code that at least warns if a sizeof() entry is missing #15173

Open p5pRT opened 8 years ago

p5pRT commented 8 years ago

Migrated from rt.perl.org#127517 (status was 'open')

Searchable as RT127517$

p5pRT commented 8 years ago

From @kentfredric

Created by @kentfredric

TL;DR Version​:

  $sizeof{'struct fiemap'} # should go bang in generated code if nobody has set it yet   # because 0 + 'sizeof FOO' returning '0' is rarely\, if ever\, useful.

And while we're at it\, a better coverage of this issue and how to manually resolve it would be a helpful addition to h2ph's documentation\, which the "please fix this" error should point to.

Something like​:

  echo 'void main(){ printf("%#lx\n"\, sizeof(struct fiemap)); }' |\   gcc -x c -include stdio.h -include linux/fiemap.h - -o /tmp/a.out && /tmp/a.out

  0x20

With instructions on where to put that value so it can be expected to work would be a start.

Grandpa Simpson Story Version​:

There's a nice bug that you don't even know is a bug until you spend a bunch of time wondering why

  ioctl( $handle\, FS_IO_FIEMAP\, $buffer );

is dying with "Invalid ioctl" stuff.

Its documented in `ioctl` that using `.ph` files is the "recommended" option\, it says "non trivial"\, but doesn't say "Probably broken by default and silently broken at that".

--[ h2ph ]-- BUGS   Doesn't construct the %sizeof array for you. ----

This doesn't seem obvious that it would be a problem\, until you go digging though the .ph files and realise FS_IO_FIEMAP ( and a huge number of other ioctls ) are implemented in terms of one of the following defines​:

--[ /usr/include/asm-generic/ioctl.h ]-- #define _IOC_TYPECHECK(t) (sizeof(t))

/* used to create numbers */ #define _IO(type\,nr) _IOC(_IOC_NONE\,(type)\,(nr)\,0) #define _IOR(type\,nr\,size) _IOC(_IOC_READ\,(type)\,(nr)\,(_IOC_TYPECHECK(size))) #define _IOW(type\,nr\,size) _IOC(_IOC_WRITE\,(type)\,(nr)\,(_IOC_TYPECHECK(size))) #define _IOWR(type\,nr\,size) _IOC(_IOC_READ|_IOC_WRITE\,(type)\,(nr)\,(_IOC_TYPECHECK(size))) #define _IOR_BAD(type\,nr\,size) _IOC(_IOC_READ\,(type)\,(nr)\,sizeof(size)) #define _IOW_BAD(type\,nr\,size) _IOC(_IOC_WRITE\,(type)\,(nr)\,sizeof(size)) #define _IOWR_BAD(type\,nr\,size) _IOC(_IOC_READ|_IOC_WRITE\,(type)\,(nr)\,sizeof(size))

--- grep -E '#define.*\s_IO(C|R|W|WR)' -r /usr/include/linux/ | wc -l 1033

grep -E '^#define.*\s_IO(C|R|W|WR)' -r /usr/include/ | wc -l 1461 ----

This block of code here​: https://metacpan.org/source/SHAY/perl-5.22.1/utils/h2ph.PL#L445-468

Merely maps

  sizeof(struct foo)

to

  $sizeof{'struct foo'}

But does not guard against the likely possibility that value is undefined a the time the generated symbol is evaluated.

More over\, as generated symbols are compound expressions\, there's no way to tell an undef happened by looking at what they return\, you have to know what the expected value is and compare the expected value with the one you got and go "that's wrong" and then work out why.

  #define SAMPLE_SZ 5 + (sizeof sample)

perl​: say SAMPLE_SZ; # 5 C​: printf "%ld\n"\, SAMPLE_SZ; # 13

Which in the case of FS_IOC_FIEMAP is quite a few layers deep\, because the innocuous

  #define FS_IOC_FIEMAP _IOWR('f'\, 11\, struct fiemap)

Turns into this under the C-Preprocessor​:

  (((2U|1U) \<\< (((0 +8)+8)+14)) | ((('f')) \<\< (0 +8)) | (((11)) \<\< 0) | ((((sizeof(struct fiemap)))) \<\< ((0 +8)+8)))

The attached files demonstrate this issue​:

1. echo 'int main(){ printf("%ld\n"\, sample_sz); return 0;}' | gcc -include stdio.h -include test.h -x c - -o test.o && ./test.o

13

2. mkdir out && h2ph -d out test.h && perl test.pl

5

Given the generated code by h2ph silently emits an entirely wrong constant\, it would be better if it gave up and demanded somebody provide the appropriate values of $sizeof{} somehow.

Now\, about that onion in my belt ....

Perl Info ``` Flags: category=utilities severity=medium Site configuration information for perl 5.22.1: Configured by kent at Fri Dec 25 11:17:30 NZDT 2015. Summary of my perl5 (revision 5 version 22 subversion 1) configuration: Platform: osname=linux, osvers=4.3.0-gentoo, archname=x86_64-linux uname='linux katipo2 4.3.0-gentoo #31 smp preempt sat nov 14 15:23:26 nzdt 2015 x86_64 intel(r) core(tm) i5-2410m cpu @ 2.30ghz genuineintel gnulinux ' config_args='-de -Dprefix=/home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc -Doptimize= -fno-stack-protector -O3 -march=native -mtune=native -Dman1dir=none -Dman3dir=none -Accflags= -fno-stack-protector -DPERL_HASH_FUNC_SDBM -DPERL_DISABLE_PMC -Aldflags= -fno-stack-protector -Aeval:scriptdir=/home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/bin' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-stack-protector -DPERL_HASH_FUNC_SDBM -DPERL_DISABLE_PMC -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize=' -fno-stack-protector -O3 -march=native -mtune=native', cppflags='-fno-stack-protector -DPERL_HASH_FUNC_SDBM -DPERL_DISABLE_PMC -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong' ccversion='', gccversion='5.2.0', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc', ldflags =' -fno-stack-protector -fstack-protector-strong -L/usr/local/lib' libpth=/usr/lib/gcc/x86_64-pc-linux-gnu/5.2.0/include-fixed /usr/lib /usr/local/lib /lib/../lib64 /usr/lib/../lib64 /lib /lib64 /usr/lib64 /usr/local/lib64 libs=-lpthread -lnsl -lnm -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -lnsl -lnm -ldl -lm -lcrypt -lutil -lc libc=libc-2.22.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.22' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -fno-stack-protector -O3 -march=native -mtune=native -L/usr/local/lib -fstack-protector-strong' @INC for perl 5.22.1: /home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/lib/site_perl/5.22.1/x86_64-linux /home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/lib/site_perl/5.22.1 /home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/lib/5.22.1/x86_64-linux /home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/lib/5.22.1 . Environment for perl 5.22.1: HOME=/home/kent LANG=en_NZ.UTF8 LANGUAGE (unset) LC_CTYPE=en_NZ.UTF8 LC_TIME=en_NZ.UTF8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/kent/perl5/perlbrew/bin:/home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/bin:/home/kent/.perl6/2013.04/bin:/home/kent/.gem/ruby/1.8/bin/:/home/kent/.rvm/gems/ruby-2.1.2/bin:/home/kent/.rvm/gems/ruby-2.1.2@global/bin:/home/kent/.rvm/rubies/ruby-2.1.2/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/5.3.0:/usr/games/bin:/home/kent/.rvm/bin:/home/kent/.rvm/bin PERLBREW_BASHRC_VERSION=0.74 PERLBREW_HOME=/home/kent/.perlbrew PERLBREW_MANPATH=/home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/man PERLBREW_PATH=/home/kent/perl5/perlbrew/bin:/home/kent/perl5/perlbrew/perls/5.22.1-nossp-sdbm-nopmc/bin PERLBREW_PERL=5.22.1-nossp-sdbm-nopmc PERLBREW_ROOT=/home/kent/perl5/perlbrew PERLBREW_VERSION=0.74 PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 8 years ago

From @kentfredric

test.h

p5pRT commented 8 years ago

From @kentfredric

test.pl

p5pRT commented 8 years ago

From @jkeenan

On Thu Feb 11 05​:00​:42 2016\, kentfredric wrote​:

This is a bug report for perl from kentnl@​cpan.org\, generated with the help of perlbug 1.40 running under perl 5.22.1.

----------------------------------------------------------------- [Please describe your issue here]

TL;DR Version​:

$sizeof{'struct fiemap'} # should go bang in generated code if nobody has set it yet # because 0 + 'sizeof FOO' returning '0' is rarely\, if ever\, useful.

And while we're at it\, a better coverage of this issue and how to manually resolve it would be a helpful addition to h2ph's documentation\, which the "please fix this" error should point to.

Something like​:

echo 'void main(){ printf("%#lx\n"\, sizeof(struct fiemap)); }' |\ gcc -x c -include stdio.h -include linux/fiemap.h - -o /tmp/a.out && /tmp/a.out

0x20

With instructions on where to put that value so it can be expected to work would be a start.

Grandpa Simpson Story Version​:

There's a nice bug that you don't even know is a bug until you spend a bunch of time wondering why

ioctl( $handle\, FS_IO_FIEMAP\, $buffer );

is dying with "Invalid ioctl" stuff.

Its documented in `ioctl` that using `.ph` files is the "recommended" option\, it says "non trivial"\, but doesn't say "Probably broken by default and silently broken at that".

--[ h2ph ]-- BUGS Doesn't construct the %sizeof array for you. ----

This doesn't seem obvious that it would be a problem\, until you go digging though the .ph files and realise FS_IO_FIEMAP ( and a huge number of other ioctls ) are implemented in terms of one of the following defines​:

--[ /usr/include/asm-generic/ioctl.h ]-- #define _IOC_TYPECHECK(t) (sizeof(t))

/* used to create numbers */ #define _IO(type\,nr) _IOC(_IOC_NONE\,(type)\,(nr)\,0) #define _IOR(type\,nr\,size) _IOC(_IOC_READ\,(type)\,(nr)\,(_IOC_TYPECHECK(size))) #define _IOW(type\,nr\,size) _IOC(_IOC_WRITE\,(type)\,(nr)\,(_IOC_TYPECHECK(size))) #define _IOWR(type\,nr\,size) _IOC(_IOC_READ|_IOC_WRITE\,(type)\,(nr)\,(_IOC_TYPECHECK(size))) #define _IOR_BAD(type\,nr\,size) _IOC(_IOC_READ\,(type)\,(nr)\,sizeof(size)) #define _IOW_BAD(type\,nr\,size) _IOC(_IOC_WRITE\,(type)\,(nr)\,sizeof(size)) #define _IOWR_BAD(type\,nr\,size) _IOC(_IOC_READ|_IOC_WRITE\,(type)\,(nr)\,sizeof(size))

--- grep -E '#define.*\s_IO(C|R|W|WR)' -r /usr/include/linux/ | wc -l 1033

grep -E '^#define.*\s_IO(C|R|W|WR)' -r /usr/include/ | wc -l 1461 ----

This block of code here​: https://metacpan.org/source/SHAY/perl- 5.22.1/utils/h2ph.PL#L445-468

Merely maps

sizeof(struct foo)

to

$sizeof{'struct foo'}

But does not guard against the likely possibility that value is undefined a the time the generated symbol is evaluated.

More over\, as generated symbols are compound expressions\, there's no way to tell an undef happened by looking at what they return\, you have to know what the expected value is and compare the expected value with the one you got and go "that's wrong" and then work out why.

#define SAMPLE_SZ 5 + (sizeof sample)

perl​: say SAMPLE_SZ; # 5 C​: printf "%ld\n"\, SAMPLE_SZ; # 13

Which in the case of FS_IOC_FIEMAP is quite a few layers deep\, because the innocuous

#define FS_IOC_FIEMAP _IOWR('f'\, 11\, struct fiemap)

Turns into this under the C-Preprocessor​:

(((2U|1U) \<\< (((0 +8)+8)+14)) | ((('f')) \<\< (0 +8)) | (((11)) \<\< 0) | ((((sizeof(struct fiemap)))) \<\< ((0 +8)+8)))

The attached files demonstrate this issue​:

1. echo 'int main(){ printf("%ld\n"\, sample_sz); return 0;}' | gcc -include stdio.h -include test.h -x c - -o test.o && ./test.o

13

2. mkdir out && h2ph -d out test.h && perl test.pl

5

Given the generated code by h2ph silently emits an entirely wrong constant\, it would be better if it gave up and demanded somebody provide the appropriate values of $sizeof{} somehow.

Now\, about that onion in my belt ....

What I got when I tried this out (renaming the files provide to reflect the RT #)​:

##### $> echo 'int main(){ printf("%ld\n"\, sample_sz); return 0;}' | gcc -include stdio.h -include 127517-test.h -x c - -o 127517-test.o && ./127517-test.o 13

$> mkdir -p out && h2ph -d out 127517-test.h && perl 127517-test.pl127517-test.h -> 127517-test.ph Undefined subroutine &main​::sample_sz called at 127517-test.pl line 6. #####

Am I missing something?

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 8 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 8 years ago

From @kentfredric

On Tue Mar 29 15​:34​:06 2016\, jkeenan wrote​:

$> mkdir -p out && h2ph -d out 127517-test.h && perl 127517- test.pl127517-test.h -> 127517-test.ph Undefined subroutine &main​::sample_sz called at 127517-test.pl line 6. #####

Am I missing something?

I suspect when you renamed the .h files from their default\, you didn't update the test.pl to reflect the new .ph name. ( which it states explicitly by filename ).

Attached is a script that\, for me\, makes it work as expected pulling directly from rt sources\, renaming to your names.

Commenting out the "sed" line replicates your issue.

p5pRT commented 8 years ago

From @kentfredric

repo.sh

p5pRT commented 8 years ago

From @kentfredric

Bah. Didn't notice minor bug in `chdir` .... what language is this again? -_-.

Should work now even when you aren't already in a dir with a fixed test.pl + test.ph :P

p5pRT commented 8 years ago

From @kentfredric

repo.sh