bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* Modules xsize and idx
@ 2021-04-07  7:57 Marc Nieper-Wißkirchen
  2021-04-07  9:51 ` Bruno Haible
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Nieper-Wißkirchen @ 2021-04-07  7:57 UTC (permalink / raw)
  To: bug-gnulib@gnu.org List

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

What is the relationship between these two modules? Both try to minimize
subtle bugs due to overflow.

However, both approaches cannot be easily combined as xsize expects
unsigned integers while idx is a signed one.

What is the suggested use of these modules for new code?

Thanks,

Marc

[-- Attachment #2: Type: text/html, Size: 890 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Modules xsize and idx
  2021-04-07  7:57 Modules xsize and idx Marc Nieper-Wißkirchen
@ 2021-04-07  9:51 ` Bruno Haible
  2021-04-07 11:00   ` Marc Nieper-Wißkirchen
  0 siblings, 1 reply; 6+ messages in thread
From: Bruno Haible @ 2021-04-07  9:51 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Marc Nieper-Wißkirchen

Hi Marc,

> What is the relationship between these two modules? Both try to minimize
> subtle bugs due to overflow.

These two modules, and the wraparound/overflow checking macros of 'intprops'
[1], are attempts to catch integer overflow.

The three approaches differ in terms of coding effort and percentage
of overflows that get caught.

With 'idx', you use signed integers, and rely on compiler options such as
'gcc -ftrapv' or 'gcc -fsanitize=undefined' to report overflows.
  - Coding effort: small.
  - Overflows caught: all.

With 'xsize', you use unsigned integers (size_t), and do a single overflow
check at the end of the computation; this check is implicit if you call
malloc, as malloc (SIZE_MAX) will always fail.
  - Coding effort: medium.
  - Overflows caught: those with explicit checks.

With 'intprops', you use signed or unsigned integers, and do an overflow
check at each step of the computation.
  - Coding effort: high.
  - Overflows caught: those with explicit checks.

> However, both approaches cannot be easily combined as xsize expects
> unsigned integers while idx is a signed one.

You don't need combine the three approaches for the same computation.
For each computation, pick the approach you prefer.

> What is the suggested use of these modules for new code?

IMO, there's no definite answer to this question. All three approaches are,
in some way, experimental. At least as long as not all distros are
compiling with 'gcc -ftrapv' systematically.

Paul, how do you see this?

(I'm thinking of adding the answers to the documentation.)

Bruno

[1] https://www.gnu.org/software/gnulib/manual/html_node/Wraparound-Arithmetic.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Modules xsize and idx
  2021-04-07  9:51 ` Bruno Haible
@ 2021-04-07 11:00   ` Marc Nieper-Wißkirchen
  2021-04-07 18:28     ` Paul Eggert
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Nieper-Wißkirchen @ 2021-04-07 11:00 UTC (permalink / raw)
  To: Bruno Haible; +Cc: Marc Nieper-Wißkirchen, bug-gnulib@gnu.org List

[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]

Hi Bruno,

thanks for replying so quickly.

Let's assume I have a procedure

void *foo_create (size_t n)
{
  void *foo = malloc (a + n * b);
  if (foo == NULL) ...;
  ...
  return foo;
}

I want 'foo_create' to handle possible overflows. To me, it seems that
should use the xsize module for this and to replace 'a + n * b' accordingly
because I have no easy local control over "-ftrapv". This seems to force,
however, that the 'n' parameter of the 'foo_create' has to be a size_t and
not an idx_t. Unless I want possibly unsafe casts in my program, this
forces the code interacting with 'foo_create' to use 'size_t' instead of
'idx_t' as well, which somewhat seems to forfeit the advantages of 'idx_t'.

That's why I am wondering whether it makes sense to have an xsize module
that uses idx_t instead of size_t.

PS Another question related to idx_t: Often I code something like:

void f (size_t n, size_t i)
{
  assure (i < n);
  ...
}

Now if I replaced unsigned by signed ints, I guess I should write

void f (idx_t n, idx_t i)
{
  assure (i < n);
  assure (0 <= i);
  ...
}

But this makes the code more complicated... :(


Am Mi., 7. Apr. 2021 um 11:51 Uhr schrieb Bruno Haible <bruno@clisp.org>:

> Hi Marc,
>
> > What is the relationship between these two modules? Both try to minimize
> > subtle bugs due to overflow.
>
> These two modules, and the wraparound/overflow checking macros of
> 'intprops'
> [1], are attempts to catch integer overflow.
>
> The three approaches differ in terms of coding effort and percentage
> of overflows that get caught.
>
> With 'idx', you use signed integers, and rely on compiler options such as
> 'gcc -ftrapv' or 'gcc -fsanitize=undefined' to report overflows.
>   - Coding effort: small.
>   - Overflows caught: all.
>
> With 'xsize', you use unsigned integers (size_t), and do a single overflow
> check at the end of the computation; this check is implicit if you call
> malloc, as malloc (SIZE_MAX) will always fail.
>   - Coding effort: medium.
>   - Overflows caught: those with explicit checks.
>
> With 'intprops', you use signed or unsigned integers, and do an overflow
> check at each step of the computation.
>   - Coding effort: high.
>   - Overflows caught: those with explicit checks.
>
> > However, both approaches cannot be easily combined as xsize expects
> > unsigned integers while idx is a signed one.
>
> You don't need combine the three approaches for the same computation.
> For each computation, pick the approach you prefer.
>
> > What is the suggested use of these modules for new code?
>
> IMO, there's no definite answer to this question. All three approaches are,
> in some way, experimental. At least as long as not all distros are
> compiling with 'gcc -ftrapv' systematically.
>
> Paul, how do you see this?
>
> (I'm thinking of adding the answers to the documentation.)
>
> Bruno
>
> [1]
> https://www.gnu.org/software/gnulib/manual/html_node/Wraparound-Arithmetic.html
>
>

[-- Attachment #2: Type: text/html, Size: 6061 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Modules xsize and idx
  2021-04-07 11:00   ` Marc Nieper-Wißkirchen
@ 2021-04-07 18:28     ` Paul Eggert
  2021-04-07 21:01       ` xalloc.h use idx_t Bruno Haible
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Eggert @ 2021-04-07 18:28 UTC (permalink / raw)
  To: Marc Nieper-Wißkirchen, Bruno Haible; +Cc: bug-gnulib@gnu.org List

On 4/7/21 4:00 AM, Marc Nieper-Wißkirchen wrote:

> That's why I am wondering whether it makes sense to have an xsize module
> that uses idx_t instead of size_t.

It might, yes. I use intprops.h for this sort of thing, but perhaps a 
stripped-down header would be appropriate.

I am planning to make xalloc.h use idx_t rather than size_t for object 
and byte counts, as we really should be using signed integers there, for 
all the usual reasons. So in some sense I want xsize's current API to be 
obsolete.

>    assure (i < n);
>    assure (0 <= i);

I suggest writing 'assure (0 <= i && i < n);' as it's easier to read, at 
least for me. Better yet, use 'affirm' rather than 'assure' as this 
helps the compiler generate better code.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: xalloc.h use idx_t
  2021-04-07 18:28     ` Paul Eggert
@ 2021-04-07 21:01       ` Bruno Haible
  2021-04-07 21:12         ` Marc Nieper-Wißkirchen
  0 siblings, 1 reply; 6+ messages in thread
From: Bruno Haible @ 2021-04-07 21:01 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Marc Nieper-Wißkirchen, bug-gnulib

Hi Paul,

> I am planning to make xalloc.h use idx_t rather than size_t for object 
> and byte counts, as we really should be using signed integers there, for 
> all the usual reasons.

I agree that using idx_t in more places helps reduce overflow problem.

However, since 'xalloc' started out as "malloc() which can't return NULL",
this would introduce an inconsistency w.r.t. malloc().
Could programmers still replace calls to malloc() with calls to xmalloc()
without thinking, without considering the context?
And vice versa, when transforming code into library code, can programmers
still replace calls to xmalloc() with calls to malloc() and a NULL check,
mechanically?

(I hope the answer is "yes", but maybe I'm overlooking something?)

Bruno



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: xalloc.h use idx_t
  2021-04-07 21:01       ` xalloc.h use idx_t Bruno Haible
@ 2021-04-07 21:12         ` Marc Nieper-Wißkirchen
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Nieper-Wißkirchen @ 2021-04-07 21:12 UTC (permalink / raw)
  To: Bruno Haible
  Cc: Marc Nieper-Wißkirchen, Paul Eggert, bug-gnulib@gnu.org List

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

xalloc is now called with a size_t argument. If the argument type is
silently changed to an idx_t, existing code which calls the new xalloc
still with a size_t argument will trigger a compiler warning under GCC's
-Wsign-conversion.

Fixing existing code isn't easy because the sizeof operator returns a
size_t, which somehow would have to be converted to an idx_t without
triggering any compiler warnings.

The coexistence of size_t and idx_t is problematic. If we want to drop
size_t somewhere, we have to drop it everywhere it seems. This includes
providing a signed sizeof operator and modules like flexmember may have to
revisited as well.

Marc

Am Mi., 7. Apr. 2021 um 23:01 Uhr schrieb Bruno Haible <bruno@clisp.org>:

> Hi Paul,
>
> > I am planning to make xalloc.h use idx_t rather than size_t for object
> > and byte counts, as we really should be using signed integers there, for
> > all the usual reasons.
>
> I agree that using idx_t in more places helps reduce overflow problem.
>
> However, since 'xalloc' started out as "malloc() which can't return NULL",
> this would introduce an inconsistency w.r.t. malloc().
> Could programmers still replace calls to malloc() with calls to xmalloc()
> without thinking, without considering the context?
> And vice versa, when transforming code into library code, can programmers
> still replace calls to xmalloc() with calls to malloc() and a NULL check,
> mechanically?
>
> (I hope the answer is "yes", but maybe I'm overlooking something?)
>
> Bruno
>
>

[-- Attachment #2: Type: text/html, Size: 2361 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-07 21:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07  7:57 Modules xsize and idx Marc Nieper-Wißkirchen
2021-04-07  9:51 ` Bruno Haible
2021-04-07 11:00   ` Marc Nieper-Wißkirchen
2021-04-07 18:28     ` Paul Eggert
2021-04-07 21:01       ` xalloc.h use idx_t Bruno Haible
2021-04-07 21:12         ` Marc Nieper-Wißkirchen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).