[syslinux] EXTLINUX - GCC 5
Thomas Schmitt
scdbackup at gmx.net
Sat Jul 11 03:11:48 PDT 2015
Hi,
Gene Cumm wrote:
> > 3) It feels like this is a moving target where gcc keeps changing and
> > different results get reported.
Do we have indications that different versions of gcc5
cause different behavior on the same build and boot machines ?
Ady wrote:
> Since the issue is only present on specific
> hardware / firmware, whatever might seem to "solve" the behavior in one
> system might not be really a solution for others.
One should at least determine whether the problem is with
the machines which build SYSLINUX or with those which try
to boot it.
(E.g. is a ISO with gcc5-built ISOLINUX bootable on some
machines and not on others ?
Does one machine build good ISOs with gcc5 and the other
produces bad ones ?)
My best theory is still that SYSLINUX would have some C code
parts which allow the compilers to produce non-equivalent
machine code.
I see complaints in the output of cppcheck 1.67, which could
match this criterium.
(Maybe one should subscribe SYSLINUX to Coverty Scan ?)
-----------------------------------------------------------------
[com32/gpllib/dmi/dmi_memory.c:205]: (error) Undefined behavior: Variable 'type' is used as parameter and destination in s[n]printf().
[com32/gpllib/dmi/dmi_memory.c:217]: (error) Undefined behavior: Variable 'connection' is used as parameter and destination in s[n]printf().
[com32/gpllib/dmi/dmi_memory.c:247]: (error) Undefined behavior: Variable 'size' is used as parameter and destination in s[n]printf().
[com32/gpllib/dmi/dmi_memory.c:249]: (error) Undefined behavior: Variable 'size' is used as parameter and destination in s[n]printf().
The complaint says it all: It is an invitation for trouble to
use the same memory as destination and source of a copy
operation.
-----------------------------------------------------------------
[com32/gpllib/dmi/dmi.c:322] -> [com32/gpllib/dmi/dmi.c:321]: (warning) Array 'type[13]' accessed at index 13, which is out of bounds. Otherwise condition 'code<=13' is redundant.
is caused by missing subtraction of 1 in
strlcpy(dmi->base_board.type, type[code],
It should be
strlcpy(dmi->base_board.type, type[code - 1],
-----------------------------------------------------------------
[com32/gpllib/dmi/dmi_processor.c:381] -> [com32/gpllib/dmi/dmi_processor.c:380]: (warning) Array 'upgrade[23]' accessed at index 24, which is out of bounds. Otherwise condition 'code<=25' is redundant.
is caused by missing commas in lines
"Socket 939" /* 0x12 */
"Socket LGA1366" /* 0x19 */
-----------------------------------------------------------------
[com32/hdt/hdt-cli.c:609]: (error) Possible null pointer dereference: argv
Hard to say whether this is really wrong code.
It depends on the two while-loops in parse_command_line() whether
the second allocates as many items as the the first one predicted.
It is very optimistic about success of malloc().
-----------------------------------------------------------------
[com32/hdt/hdt-cli.c:90]: (error) Memory is allocated but not initialized: new
Might be a false positive.
-----------------------------------------------------------------
[com32/lib/jpeg/tinyjpeg.c:314] -> [com32/lib/jpeg/tinyjpeg.c:309]: (warning) Array 'DCT[64]' accessed at index 64, which is out of bounds. Otherwise condition 'j>=64' is redundant.
Probably a false positive.
(What is the reason of macro "__unlikely()" which is defined as "(!!(x))" ?)
-----------------------------------------------------------------
[com32/lib/syslinux/pxe_dns.c:56]: (error) Uninitialized variable: q
False positive.
cppcheck seems to not understand the use of the return value of sscanf().
-----------------------------------------------------------------
[com32/libupload/upload_tftp.c:138]: (error) Uninitialized struct member: tftp.srv_ip
Looks like a valid complaint.
But from where to get a valid tftp.srv_ip ?
-----------------------------------------------------------------
[com32/modules/prdhcp.c:145]: (error) Uninitialized variable: p
Possibly a false positive.
It depends on what pxe_get_cached_info() does to &p.
-----------------------------------------------------------------
Checking com32/rosh/rosh.c: COMMAND_LINE_SIZE...
[com32/rosh/rosh.c:1128]: (error) Uninitialized variable: cmdstr
Checking com32/rosh/rosh.c: FILENAME_MAX...
[com32/rosh/rosh.c:559]: (error) Uninitialized variable: filestr2
...
[com32/rosh/rosh.c:600]: (error) Uninitialized variable: filestr2
Possibly non-functional compile time alternatives due to
unqualified macro settings by cppcheck.
-----------------------------------------------------------------
[com32/sysdump/memmap.c:50]: (error) Possible null pointer dereference: buf
[com32/sysdump/memmap.c:51]: (error) Possible null pointer dereference: buf
[com32/sysdump/memmap.c:52]: (error) Possible null pointer dereference: buf
Probably false positive.
cppcheck has problems to recognize memory allocation on demand.
-----------------------------------------------------------------
[core/fs/pxe/dnsresolv.c:107]: (error) Uninitialized variable: ip
Probably false positive.
-----------------------------------------------------------------
[core/fs/xfs/xfs_dir2.c:588]: (error) Uninitialized variable: irec
...
[core/fs/xfs/xfs_dir2.c:584]: (error) Uninitialized struct member: irec.br_blockcount
Hard to say.
Who understands XFS structure ?
-----------------------------------------------------------------
Checking core/lwip/src/core/ipv4/igmp.c: LWIP_DEBUG;LWIP_IGMP...
[core/lwip/src/core/ipv4/igmp.c:813]: (error) Possible null pointer dereference: igmp
Looks like a valid complaint.
Will cause SIGSEGV in case of pbuf_alloc() returning 0.
Question is whether the macro combination is supposed to be
functional.
-----------------------------------------------------------------
Checking core/lwip/src/core/ipv4/igmp.c: LWIP_DEBUG;LWIP_IGMP...
[core/lwip/src/core/ipv4/igmp.c:397]: (error) Uninitialized variable: igmp
Hard to say.
Possibly a non-functional macro combination.
-----------------------------------------------------------------
[core/serirq.c:203]: (error) Null pointer dereference
Looks like confusion of memcpy() and memset():
memcpy(SerialHead, 0x0, serial_buf_size);
should probably be
memset(SerialHead, 0x0, serial_buf_size);
-----------------------------------------------------------------
[gpxe/src/core/iobuf.c:50]: (error) Possible null pointer dereference: iobuf
...
[gpxe/src/core/iobuf.c:51]: (error) Null pointer dereference
Probably a false positive.
cppcheck seems not to understand __alignof__() which does not
dereference *iobuf but just inspects its type.
-----------------------------------------------------------------
[gpxe/src/core/uri.c:154]: (error) Possible null pointer dereference: authority
Indeed the code path through "else" of
/* Identify net/absolute/relative path */
if ( strncmp ( path, "//", 2 ) == 0 ) {
does not set variable authority.
-----------------------------------------------------------------
[gpxe/src/crypto/axtls/aes.c:251]: (error) Uninitialized variable: t1
Crypto stuff is cryptic by intention.
-----------------------------------------------------------------
[gpxe/src/drivers/net/e1000/e1000_hw.c:7395]: (error) Uninitialized variable: phy_data
...
[gpxe/src/drivers/net/e1000/e1000_hw.c:7557]: (error) Uninitialized variable: phy_data
Might be a false positive.
The code path which does not set phy_data might be separated from
the code paths which use it.
-----------------------------------------------------------------
[gpxe/src/util/efirom.c:68]: (error) Uninitialized variable: len
A function which is intended to work by clairvoyance ?
static size_t file_size ( FILE *file ) {
ssize_t len;
return len;
}
(Its quality as random number generator is mediocre, too.
There's too much redundancy in stack usage.)
-----------------------------------------------------------------
Checking gpxe/src/util/elf2efi.c...
[gpxe/src/util/elf2efi.c:352]: (error) Uninitialized variable: data_start
Checking gpxe/src/util/elf2efi.c: MDE_CPU_IA32...
Checking gpxe/src/util/elf2efi.c: MDE_CPU_X64...
Something new: A non-functional code branch if no macro is set.
-----------------------------------------------------------------
Checking gpxe/src/util/nrv2b.c: ENCODE...
[gpxe/src/util/nrv2b.c:634]: (error) Array 's.best_pos[1]' accessed at index 2, which is out of bounds.
[gpxe/src/util/nrv2b.c:635]: (error) Array 's.best_pos[1]' accessed at index 2, which is out of bounds.
Clearly a hardcoded wrong index.
#define SWD_BEST_OFF 1
...
struct ucl_swd
{
...
#if defined(SWD_BEST_OFF)
unsigned int best_pos[ SWD_BEST_OFF ];
#endif
...
};
...
#if defined(SWD_BEST_OFF)
if (s->best_pos[2] == 0)
s->best_pos[2] = key + 1;
#endif
Shall the index be [SWD_BEST_OFF - 1] ?
Shall this be done only if SWD_BEST_OFF > 2 ?
-----------------------------------------------------------------
Checking linux/syslinux.c: __KLIBC__...
[linux/syslinux.c:123]: (error) Uninitialized variable: loop_fd
Valid complaint:
/*
* If DO_DIRECT_MOUNT is 0, call mount(8)
* If DO_DIRECT_MOUNT is 1, call mount(2)
*/
#ifdef __KLIBC__
# define DO_DIRECT_MOUNT 1
#else
# define DO_DIRECT_MOUNT 0 /* glibc has broken losetup ioctls */
#endif
...
#if DO_DIRECT_MOUNT
{
if (!S_ISBLK(st.st_mode)) {
/* It's file, need to mount it loopback */
...
int loop_fd;
for (n = 0; loop_fd < 0; n++) {
...
if (ioctl(loop_fd, LOOP_SET_FD, (void *)dev_fd)) {
losetup ioctls have few chance if they get called by random decision.
This would be a nice suspect if __KLIBC__ is defined.
-----------------------------------------------------------------
[win/syslinux.c:506]: (error) Uninitialized variable: sdn
Possibly a false positive.
-----------------------------------------------------------------
Have a nice day :)
Thomas
More information about the Syslinux
mailing list