On 2015-10-05 21:45, Mark Wielaard wrote:
On Fri, Oct 02, 2015 at 12:10:47AM +0300, Alexander Cherepanov wrote:
On 2015-09-16 18:25, Mark Wielaard wrote:
On Fri, 2015-09-11 at 12:22 -0700, Roland McGrath wrote:
It looks fine to me from a quick skim, but Mark should review and test it too.

I am not super enthusiastic about this change, it seems to just take
away type/size information that the compiler/bounds checking tools can
use.

I'm not sure I fully understand the problem. As I understand it unions of
VLAs are not Ok while VLAs are Ok and even desirable due to bounds checking,
right?

There are two issues. First with GCC VLA types are OK and desired compared
to types not using bounds. But if such a VLAs might be unbounded then they
should not be stack allocated (this is enforced by building with
-Wstack-usage except for a couple of files that haven't been converted).

Great.

Secondly there are people who want to use clang to build elfutils.

Yes, this nice to have.

clang
doesn't support various GNU extensions used in the code like VLAs. So
for those people any VLA type seems problematic.

VLAs are in C99 (but optional in C11) and are supported by clang[1]. But VLAs inside structs/unions are prohibited by the standards, are a gcc extension and are not supported by clang.

[1] http://clang.llvm.org/compatibility.html#vla

Why not just use VLAs of unions? Cold memory?

I am not sure clang supports such VLA types.

Yes, it should work.

If it does, then I would say
VLAs of unions are preferred above a type that doesn't include bounds.

In comparison with a pointer-to-array (classical Pascal) approach it has pros and cons.

Pros:
- shorter declaration (no malloc, no calculations).
Cons:
- bounded arrays only;
- requires more memory (for 32-bit case);
- non-continuous memory (for 32-bit case).

Not sure if these cons are important.

Given that the current approach (before the patch) already required to write
superfluous "->" perhaps an approach requiring a superfluous "*" will fit?
Like this:

   void *data = malloc (...);
   T32 (*a32)[n] = data;
   T64 (*a64)[n] = data;

Then the use looks like "(*a32)[i].member". Clang seems to be happy and its
UBSAN works fine.

If that works that would probably be preferred since then ubsan can see
the array bounds and help catch issues.

I believe it was not working in gcc 4.9. It works in newer gcc?

You can build and run elfutils
and the tests with configure --enable-sanitize-undefined to use ubsan
checking.

Nice.

--
Alexander Cherepanov

Reply via email to