Re: memset bugs.
On 11/27/2009 02:25 PM, Casey Dahlin wrote: On 11/27/2009 06:03 AM, Richard W.M. Jones wrote: On Fri, Nov 27, 2009 at 03:28:19AM -0500, Gregory Maxwell wrote: A literal zero prior to preprocessing is either a bug, or some kind of dead- code causing place-holder. Not necessarily .. the C code itself may be generated from something else. Rich. In which case the C code is no longer source and should be excluded from the analysis. No, when swig (or whatever) produces bad code, we still want the compiler to identify it and toss it. It's then up to the packager to realize it's swig producing the bad code, but it isn't magically good code that doesn't result in real bugs. -- Peter I'd like to start a religion. That's where the money is. -- L. Ron Hubbard to Lloyd Eshbach, in 1949; quoted by Eshbach in _Over My Shoulder_. -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On 11/30/2009 10:39 AM, Peter Jones wrote: On 11/27/2009 02:25 PM, Casey Dahlin wrote: On 11/27/2009 06:03 AM, Richard W.M. Jones wrote: On Fri, Nov 27, 2009 at 03:28:19AM -0500, Gregory Maxwell wrote: A literal zero prior to preprocessing is either a bug, or some kind of dead- code causing place-holder. Not necessarily .. the C code itself may be generated from something else. Rich. In which case the C code is no longer source and should be excluded from the analysis. No, when swig (or whatever) produces bad code, we still want the compiler to identify it and toss it. It's then up to the packager to realize it's swig producing the bad code, but it isn't magically good code that doesn't result in real bugs. The compiler isn't doing these checks, but point taken. On a tangent, what of these checks if any should be put into the compiler? Compile-time bounds checking of library functions is kind of magical and un-C-like, but its not unprecedented (printf argument checking for example). --CJD -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On 11/30/2009 11:39 AM, Casey Dahlin wrote: On 11/30/2009 10:39 AM, Peter Jones wrote: On 11/27/2009 02:25 PM, Casey Dahlin wrote: On 11/27/2009 06:03 AM, Richard W.M. Jones wrote: On Fri, Nov 27, 2009 at 03:28:19AM -0500, Gregory Maxwell wrote: A literal zero prior to preprocessing is either a bug, or some kind of dead- code causing place-holder. Not necessarily .. the C code itself may be generated from something else. Rich. In which case the C code is no longer source and should be excluded from the analysis. No, when swig (or whatever) produces bad code, we still want the compiler to identify it and toss it. It's then up to the packager to realize it's swig producing the bad code, but it isn't magically good code that doesn't result in real bugs. The compiler isn't doing these checks, but point taken. Go read Jakub's reply again ;) https://www.redhat.com/archives/fedora-devel-list/2009-November/msg01966.html -- Peter Sanity's just a one trick pony anyway. You only get one trick -- rational thinking -- but when you're good and crazy, the sky's the limit! -- The Tick -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On 11/30/2009 01:10 PM, Peter Jones wrote: On 11/30/2009 11:39 AM, Casey Dahlin wrote: On 11/30/2009 10:39 AM, Peter Jones wrote: On 11/27/2009 02:25 PM, Casey Dahlin wrote: On 11/27/2009 06:03 AM, Richard W.M. Jones wrote: On Fri, Nov 27, 2009 at 03:28:19AM -0500, Gregory Maxwell wrote: A literal zero prior to preprocessing is either a bug, or some kind of dead- code causing place-holder. Not necessarily .. the C code itself may be generated from something else. Rich. In which case the C code is no longer source and should be excluded from the analysis. No, when swig (or whatever) produces bad code, we still want the compiler to identify it and toss it. It's then up to the packager to realize it's swig producing the bad code, but it isn't magically good code that doesn't result in real bugs. The compiler isn't doing these checks, but point taken. Go read Jakub's reply again ;) https://www.redhat.com/archives/fedora-devel-list/2009-November/msg01966.html I stand corrected. --CJD -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On Fri, Nov 27, 2009 at 1:26 AM, Roopesh Majeti roopesh.maj...@gmail.com wrote: Hi All, Iam new to this fedora world.. a small question on the below discussion: It is mentioned that having, zero in the third argument is legitimate use cases. Can somebody direct me to such a use case, as i feel, giving memset a zero, is asking it, not to do anything [ might have side effects, not sure from my end, though ]. It's legitimate because the zero may ultimately be derived from macro values and restructuring the code to avoid the memset depending on defined values may be non-trivial or even impossible. John Reiser provided a good example: http://www.linux-archive.org/fedora-development/286221-memset-bugs.html Where its not a programming bug the memset(,,0) is harmless: GCC optimizes it out completely. A literal zero prior to preprocessing is either a bug, or some kind of dead- code causing place-holder. -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On Fri, Nov 27, 2009 at 03:28:19AM -0500, Gregory Maxwell wrote: A literal zero prior to preprocessing is either a bug, or some kind of dead- code causing place-holder. Not necessarily .. the C code itself may be generated from something else. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On 11/27/2009 06:03 AM, Richard W.M. Jones wrote: On Fri, Nov 27, 2009 at 03:28:19AM -0500, Gregory Maxwell wrote: A literal zero prior to preprocessing is either a bug, or some kind of dead- code causing place-holder. Not necessarily .. the C code itself may be generated from something else. Rich. In which case the C code is no longer source and should be excluded from the analysis. --CJD -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On Wed, 2009-11-25 at 23:39 -0500, Tom Lane wrote: John Reiser jrei...@bitwagon.com writes: On 11/25/2009 02:03 PM, Dave Jones wrote: A zero sized memset is always a bug. No, memset(,,0) is not always a bug. I think it's reasonably safe to assume that a *literal constant* zero in the third argument is a bug. Whether the header macros can distinguish that from compile-time-constant expressions is an interesting question, but if they can, +1 for throwing an error. I logged some time ago a trivial patch of https://bugzilla.redhat.com/show_bug.cgi?id=532492 which is apparently in rawhide now to generally avoid a warning on memset(foo, 0, 0) C. -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
Hi All, Iam new to this fedora world.. a small question on the below discussion: It is mentioned that having, zero in the third argument is legitimate use cases. Can somebody direct me to such a use case, as i feel, giving memset a zero, is asking it, not to do anything [ might have side effects, not sure from my end, though ]. Regards Roopesh M. On Thu, Nov 26, 2009 at 2:24 PM, Caolán McNamara caol...@redhat.com wrote: On Wed, 2009-11-25 at 23:39 -0500, Tom Lane wrote: John Reiser jrei...@bitwagon.com writes: On 11/25/2009 02:03 PM, Dave Jones wrote: A zero sized memset is always a bug. No, memset(,,0) is not always a bug. I think it's reasonably safe to assume that a *literal constant* zero in the third argument is a bug. Whether the header macros can distinguish that from compile-time-constant expressions is an interesting question, but if they can, +1 for throwing an error. I logged some time ago a trivial patch of https://bugzilla.redhat.com/show_bug.cgi?id=532492 which is apparently in rawhide now to generally avoid a warning on memset(foo, 0, 0) C. -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
memset bugs.
There's some obvious bugs below in a bunch of packages. The 2nd and 3rd arguments to memset calls are the wrong way around. I found these after grepping through a make prep'd devel/ tree. 15 hits out of 100G of source code isn't that bad, but we can do better! Dave Checking ./afflib/afflib-3.5.2/lib/s3_glue.cpp Found memset with swapped arguments. 303:memset(b64str,sizeof(b64str),0); Checking ./afflib/afflib-3.5.2/lib/vnode_s3.cpp Found memset with swapped arguments. 205:memset(segname,segname_len,0); Checking ./afflib/afflib-3.5.2/lib/crypto.cpp Found memset with swapped arguments. 975:memset(decrypted,total_encrypted_bytes,0); // overwrite our temp buffer Checking ./panoglview/panoglview-0.2.2/src/panocanvas.cpp Found memset with swapped arguments. 160: memset(tmp,m_maxsize*m_maxsize,0); Checking ./condor/condor-7.2.4/src/condor_c++_util/read_user_log_state.cpp Found memset with swapped arguments. 497:memset( istate-m_base_path, sizeof(istate-m_base_path), 0 ); Checking ./milkytracker/milkytracker-0.90.80/src/milkyplay/LoaderPSM.cpp Found memset with swapped arguments. 999:memset(packed,size-4+5,0); Checking ./sim/sim/sim/sockfactory.cpp Found memset with swapped arguments. 546:memset(addr, sizeof(addr), 0); Checking ./commoncpp2/commoncpp2-1.7.3/src/thread.cpp Found memset with swapped arguments. 525:memset(act, sizeof(act), 0); Checking ./commoncpp2/commoncpp2-1.7.3/src/socket.cpp Found memset with swapped arguments. 1571: memset(group, sizeof(group), 0); Checking ./celestia/celestia-1.5.1/src/celestia/winmain.cpp Found memset with swapped arguments. 2181:memset(info, sizeof(info), 0); Checking ./scummvm/scummvm-0.13.1/engines/tinsel/scene.cpp Found memset with swapped arguments. 132:memset(tempStruc, sizeof(SCENE_STRUC), 0); Checking ./aqsis/aqsis-1.6.0/tools/displays/sdcWin32/d_sdcWin32.cpp Found memset with swapped arguments. 250:memset(g_Data, sizeof(AppData), 0); Checking ./aqsis/aqsis-1.6.0/tools/displays/sdcBMP/d_sdcBMP.cpp Found memset with swapped arguments. 172:memset(g_Data, sizeof(AppData), 0); Checking ./arm4/arm4-0.8.2/src/libarm4db/berkeleydb/BerkeleyDB_report.cpp Found memset with swapped arguments. 1603: memset (summary_ptr, sizeof (*summary_ptr), 0); Checking ./arm4/arm4-0.8.2/src/libarm4db/Arm4dbDaemonSharedMemory.cpp Found memset with swapped arguments. 558:memset (stats_ptr, sizeof (*stats_ptr), 0); -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On Wed, Nov 25, 2009 at 01:43:13PM -0500, Dave Jones wrote: There's some obvious bugs below in a bunch of packages. The 2nd and 3rd arguments to memset calls are the wrong way around. I found these after grepping through a make prep'd devel/ tree. 15 hits out of 100G of source code isn't that bad, but we can do better! glibc headers warn about this (when -D_FORTIFY_SOURCE=2), so a faster way would be just grep through all packages' build.log files (preferrably on the box where they are stored to avoid downloading it all). Jakub -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On Wed, Nov 25, 2009 at 01:58:38PM -0500, Jakub Jelinek wrote: On Wed, Nov 25, 2009 at 01:43:13PM -0500, Dave Jones wrote: There's some obvious bugs below in a bunch of packages. The 2nd and 3rd arguments to memset calls are the wrong way around. I found these after grepping through a make prep'd devel/ tree. 15 hits out of 100G of source code isn't that bad, but we can do better! glibc headers warn about this (when -D_FORTIFY_SOURCE=2), so a faster way would be just grep through all packages' build.log files (preferrably on the box where they are stored to avoid downloading it all). Can we make it fail the build instead of warning ? A zero sized memset is always a bug. Dave -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
On 11/25/2009 02:03 PM, Dave Jones wrote:
On Wed, Nov 25, 2009 at 01:58:38PM -0500, Jakub Jelinek wrote:
glibc headers warn about this (when -D_FORTIFY_SOURCE=2), so a faster way
would be just grep through all packages' build.log files (preferrably on
the
box where they are stored to avoid downloading it all).
Can we make it fail the build instead of warning ?
A zero sized memset is always a bug.
No, memset(,,0) is not always a bug. A null region is not automatically a bug.
Here is one example:
struct Foo {
long x;
char hole[8 - sizeof(long)];
} foo;
memset(foo.hole, 0, sizeof(foo.hole));
On a LP64-bit machine such as x86_64, this is memset(foo.hole, 0, 0),
which is *NOT* a bug.
Perhaps the best that can be expected is for the compiler to warn
if _builtin_memset has a third argument which is known to be a compile-time
constant zero. But such a warning must be optional, for there are
legitimate use cases. Also, if the second argument to _builtin_memset
is a compile-time constant which cannot be represented in one byte
(considering both signed and unsigned cases) then another optional warning
may be appropriate.
--
--
fedora-devel-list mailing list
fedora-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-devel-list
Re: memset bugs.
John Reiser jrei...@bitwagon.com writes: On 11/25/2009 02:03 PM, Dave Jones wrote: A zero sized memset is always a bug. No, memset(,,0) is not always a bug. I think it's reasonably safe to assume that a *literal constant* zero in the third argument is a bug. Whether the header macros can distinguish that from compile-time-constant expressions is an interesting question, but if they can, +1 for throwing an error. regards, tom lane -- fedora-devel-list mailing list fedora-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/fedora-devel-list
