clang's -fcatch-undefined-behavior and detect_C89_integers() in H5detect.c:1285

Hi,

Well, it's been several years, but now I really need this fixed, so I hacked something up and am sharing the patch here.

For UBSan[1] to work, I changed the invalid casting to use memcpy():

- *((TYPE*)(_buf+align_g[_ano])) = _val; /*possible SIGBUS or SEGSEGV*/ \
+ memcpy(_buf+align_g[_ano], &_val, sizeof(TYPE)); \

For ASan[2] to work, I just skip the signal stuff.

Hopefully this (or something better?) can be merged upstream.

Cheers,

Sean

[1] <http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html&gt;
[2] <http://clang.llvm.org/docs/AddressSanitizer.html&gt;

hdf-ubsan-asan.patch (1.77 KB)

···

On Sat, 8 Sep 2012 22:09:56 -0500, Elena Pourmal said:

Sean,

Thank you for reporting! I created a ticket in our issues database
(HDFFV-8147). We will try to take a look before our next release, but
cannot promise.

If someone on FORUM will have time to investigate and have any
questions, we will be more than happy to assist (and to accept a patch :slight_smile:

Thank you!

Elena
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Sep 7, 2012, at 3:32 PM, Sean McBride wrote:

Hi all,

I have been experimenting with clang's -fcatch-undefined-behavior,

which does like its name implies. For details:

<http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html&gt;

This has detected a bug in detect_C89_integers() at H5detect.c line 1285:

DETECT_I(short, SHORT, d_g[nd_g]); nd_g++;

The DETECT_I macro is difficult to decipher, but I'm pretty sure it's

doing a left shift that's too big. Shifting a uint16_t by 16 or more
bits is undefined.

Could someone familiar with this code investigate?

It's especially annoying because this code seems to run at build time,

causing build failure:

<http://cdash.hdfgroup.uiuc.edu/viewBuildError.php?buildid=5482&gt;

Which is much more annoying than just a failing unit test or two.

Cheers,

--
____________________________________________________________
Sean McBride, B. Eng sean@rogue-research.com
Rogue Research www.rogue-research.com
Mac Software Developer Montréal, Québec, Canada

_______________________________________________
Hdf-forum is for HDF software users discussion.
Hdf-forum@hdfgroup.org
http://mail.hdfgroup.org/mailman/listinfo/hdf-forum_hdfgroup.org

Sean,

I reopened the issue; we will review the patch this Friday.

Elena

···

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Jan 25, 2016, at 3:54 PM, Sean McBride <sean@rogue-research.com<mailto:sean@rogue-research.com>> wrote:

Hi,

Well, it's been several years, but now I really need this fixed, so I hacked something up and am sharing the patch here.

For UBSan[1] to work, I changed the invalid casting to use memcpy():

- *((TYPE*)(_buf+align_g[_ano])) = _val; /*possible SIGBUS or SEGSEGV*/ \
+ memcpy(_buf+align_g[_ano], &_val, sizeof(TYPE)); \

For ASan[2] to work, I just skip the signal stuff.

Hopefully this (or something better?) can be merged upstream.

Cheers,

Sean

[1] <http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html>
[2] <http://clang.llvm.org/docs/AddressSanitizer.html>

On Sat, 8 Sep 2012 22:09:56 -0500, Elena Pourmal said:

Sean,

Thank you for reporting! I created a ticket in our issues database
(HDFFV-8147). We will try to take a look before our next release, but
cannot promise.

If someone on FORUM will have time to investigate and have any
questions, we will be more than happy to assist (and to accept a patch :slight_smile:

Thank you!

Elena
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Sep 7, 2012, at 3:32 PM, Sean McBride wrote:

Hi all,

I have been experimenting with clang's -fcatch-undefined-behavior,
which does like its name implies. For details:

<http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html>

This has detected a bug in detect_C89_integers() at H5detect.c line 1285:

DETECT_I(short, SHORT, d_g[nd_g]); nd_g++;

The DETECT_I macro is difficult to decipher, but I'm pretty sure it's
doing a left shift that's too big. Shifting a uint16_t by 16 or more
bits is undefined.

Could someone familiar with this code investigate?

It's especially annoying because this code seems to run at build time,
causing build failure:

<http://cdash.hdfgroup.uiuc.edu/viewBuildError.php?buildid=5482>

Which is much more annoying than just a failing unit test or two.

Cheers,

--
____________________________________________________________
Sean McBride, B. Eng sean@rogue-research.com<mailto:sean@rogue-research.com>
Rogue Research www.rogue-research.com<http://www.rogue-research.com>
Mac Software Developer Montréal, Québec, Canada

_______________________________________________
Hdf-forum is for HDF software users discussion.
Hdf-forum@hdfgroup.org<mailto:Hdf-forum@hdfgroup.org>
http://mail.hdfgroup.org/mailman/listinfo/hdf-forum_hdfgroup.org

<hdf-ubsan-asan.patch>

As someone who's had their patches and issues in this tracker before, not
really being able to keep an eye on them - I wanted to take this
opportunity to ask if there's been (further) consideration and movement
towards using github for both public issue tracking (possibly integrate
into your internal system, if you're using one of the more flexible
trackers) and for more open / easier tracking of the project. I wouldn't
call it successful necessarily, but I can point out a company of similar
position has done this: PrismTech's OpenSplice DDS [
https://github.com/PrismTech/opensplice ], although I think the way they
have done it can serve as an example of how not to do it :-).

Developer wise, no new stuff from me lately unfortunately. Still trying to
both make new libraries on/for HDF5 and find the projects that need HDF5.
I'm still keeping my eye on the ML though. That and waiting for some H5LT
changes to allow other languages to do refcounting, see this for more
in-depth explanation: https://github.com/h5py/h5py/issues/552

-Jason

···

On Mon, Jan 25, 2016 at 5:27 PM, Elena Pourmal <epourmal@hdfgroup.org> wrote:

Sean,

I reopened the issue; we will review the patch this Friday.

Elena
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Jan 25, 2016, at 3:54 PM, Sean McBride <sean@rogue-research.com> wrote:

Hi,

Well, it's been several years, but now I really need this fixed, so I
hacked something up and am sharing the patch here.

For UBSan[1] to work, I changed the invalid casting to use memcpy():

- *((TYPE*)(_buf+align_g[_ano])) = _val; /*possible SIGBUS or SEGSEGV*/ \
+ memcpy(_buf+align_g[_ano], &_val, sizeof(TYPE)); \

For ASan[2] to work, I just skip the signal stuff.

Hopefully this (or something better?) can be merged upstream.

Cheers,

Sean

[1] <http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html>
[2] <http://clang.llvm.org/docs/AddressSanitizer.html>

On Sat, 8 Sep 2012 22:09:56 -0500, Elena Pourmal said:

Sean,

Thank you for reporting! I created a ticket in our issues database
(HDFFV-8147). We will try to take a look before our next release, but
cannot promise.

If someone on FORUM will have time to investigate and have any
questions, we will be more than happy to assist (and to accept a patch :slight_smile:

Thank you!

Elena
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Sep 7, 2012, at 3:32 PM, Sean McBride wrote:

Hi all,

I have been experimenting with clang's -fcatch-undefined-behavior,

which does like its name implies. For details:

<http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html>

This has detected a bug in detect_C89_integers() at H5detect.c line 1285:

DETECT_I(short, SHORT, d_g[nd_g]); nd_g++;

The DETECT_I macro is difficult to decipher, but I'm pretty sure it's

doing a left shift that's too big. Shifting a uint16_t by 16 or more
bits is undefined.

Could someone familiar with this code investigate?

It's especially annoying because this code seems to run at build time,

causing build failure:

<http://cdash.hdfgroup.uiuc.edu/viewBuildError.php?buildid=5482>

Which is much more annoying than just a failing unit test or two.

Cheers,

--
____________________________________________________________
Sean McBride, B. Eng sean@rogue-research.com
Rogue Research www.rogue-research.com
Mac Software Developer Montréal, Québec, Canada

_______________________________________________
Hdf-forum is for HDF software users discussion.
Hdf-forum@hdfgroup.org
http://mail.hdfgroup.org/mailman/listinfo/hdf-forum_hdfgroup.org

<hdf-ubsan-asan.patch>

_______________________________________________
Hdf-forum is for HDF software users discussion.
Hdf-forum@lists.hdfgroup.org
http://lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org
Twitter: https://twitter.com/hdf5

Jason and all,

We have been working on moving from SVN to Git and hope to have a repository in place and open to the community at the time of the HDF5 1.10.0 release (we are targeting end of March at this point). There is no doubt that community involvement is critical to the HDF5 sustainability and code quality.

Elena

···

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Jan 25, 2016, at 4:45 PM, Jason Newton <nevion@gmail.com<mailto:nevion@gmail.com>> wrote:

As someone who's had their patches and issues in this tracker before, not really being able to keep an eye on them - I wanted to take this opportunity to ask if there's been (further) consideration and movement towards using github for both public issue tracking (possibly integrate into your internal system, if you're using one of the more flexible trackers) and for more open / easier tracking of the project. I wouldn't call it successful necessarily, but I can point out a company of similar position has done this: PrismTech's OpenSplice DDS [ https://github.com/PrismTech/opensplice ], although I think the way they have done it can serve as an example of how not to do it :-).

Developer wise, no new stuff from me lately unfortunately. Still trying to both make new libraries on/for HDF5 and find the projects that need HDF5. I'm still keeping my eye on the ML though. That and waiting for some H5LT changes to allow other languages to do refcounting, see this for more in-depth explanation: https://github.com/h5py/h5py/issues/552

-Jason

On Mon, Jan 25, 2016 at 5:27 PM, Elena Pourmal <epourmal@hdfgroup.org<mailto:epourmal@hdfgroup.org>> wrote:
Sean,

I reopened the issue; we will review the patch this Friday.

Elena
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org<http://hdfgroup.org/>
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112<tel:217.531.6112>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Jan 25, 2016, at 3:54 PM, Sean McBride <sean@rogue-research.com<mailto:sean@rogue-research.com>> wrote:

Hi,

Well, it's been several years, but now I really need this fixed, so I hacked something up and am sharing the patch here.

For UBSan[1] to work, I changed the invalid casting to use memcpy():

- *((TYPE*)(_buf+align_g[_ano])) = _val; /*possible SIGBUS or SEGSEGV*/ \
+ memcpy(_buf+align_g[_ano], &_val, sizeof(TYPE)); \

For ASan[2] to work, I just skip the signal stuff.

Hopefully this (or something better?) can be merged upstream.

Cheers,

Sean

[1] <http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html>
[2] <http://clang.llvm.org/docs/AddressSanitizer.html>

On Sat, 8 Sep 2012 22:09:56 -0500, Elena Pourmal said:

Sean,

Thank you for reporting! I created a ticket in our issues database
(HDFFV-8147). We will try to take a look before our next release, but
cannot promise.

If someone on FORUM will have time to investigate and have any
questions, we will be more than happy to assist (and to accept a patch :slight_smile:

Thank you!

Elena
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Elena Pourmal The HDF Group http://hdfgroup.org<http://hdfgroup.org/>
1800 So. Oak St., Suite 203, Champaign IL 61820
217.531.6112<tel:217.531.6112>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Sep 7, 2012, at 3:32 PM, Sean McBride wrote:

Hi all,

I have been experimenting with clang's -fcatch-undefined-behavior,
which does like its name implies. For details:

<http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html>

This has detected a bug in detect_C89_integers() at H5detect.c line 1285:

DETECT_I(short, SHORT, d_g[nd_g]); nd_g++;

The DETECT_I macro is difficult to decipher, but I'm pretty sure it's
doing a left shift that's too big. Shifting a uint16_t by 16 or more
bits is undefined.

Could someone familiar with this code investigate?

It's especially annoying because this code seems to run at build time,
causing build failure:

<http://cdash.hdfgroup.uiuc.edu/viewBuildError.php?buildid=5482>

Which is much more annoying than just a failing unit test or two.

Cheers,

--
____________________________________________________________
Sean McBride, B. Eng sean@rogue-research.com<mailto:sean@rogue-research.com>
Rogue Research www.rogue-research.com<http://www.rogue-research.com/>
Mac Software Developer Montréal, Québec, Canada

_______________________________________________
Hdf-forum is for HDF software users discussion.
Hdf-forum@hdfgroup.org<mailto:Hdf-forum@hdfgroup.org>
http://mail.hdfgroup.org/mailman/listinfo/hdf-forum_hdfgroup.org

<hdf-ubsan-asan.patch>

_______________________________________________
Hdf-forum is for HDF software users discussion.
Hdf-forum@lists.hdfgroup.org<mailto:Hdf-forum@lists.hdfgroup.org>
http://lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org
Twitter: https://twitter.com/hdf5

_______________________________________________
Hdf-forum is for HDF software users discussion.
Hdf-forum@lists.hdfgroup.org<mailto:Hdf-forum@lists.hdfgroup.org>
http://lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org
Twitter: https://twitter.com/hdf5