mbox series

[00/13] Remove a bunch of alloca uses

Message ID cover.1677533215.git.aburgess@redhat.com
Headers
Series Remove a bunch of alloca uses |

Message

Andrew Burgess Feb. 27, 2023, 9:29 p.m. UTC 
  Continuing effort to replace alloca calls with C++ data structures,
here's a bunch of places where gdb::byte_vector can be used.

This isn't all of them, there's still plenty to work through, but I
thought I'd see how this lot is received.

Thanks,
Andrew

---

Andrew Burgess (13):
  gdb: remove uses of alloca from arch-utils.c
  gdb: remove use of alloca from auxv.c
  gdb: remove use of alloca from c-lang.c
  gdb: remove use of alloca from corefile.c
  gdb: remove uses of alloca from dwarf2/expr.c
  gdb: remove a use of alloca from elfread.c
  gdb: remove use of alloca from findvar.c
  gdb: remove use of alloca from linux-nat-trad.c
  gdb: remove use of alloca from mem-break.c
  gdb: remove some uses of alloca from printcmd.c
  gdb: remove some uses of alloca from remote.c
  gdb: remove uses of alloca from valprint.c
  gdb: remove a use of alloca from symfile.c

 gdb/arch-utils.c     | 13 ++++++-------
 gdb/auxv.c           | 10 +++++-----
 gdb/c-lang.c         |  8 +++-----
 gdb/corefile.c       | 18 +++++++++---------
 gdb/dwarf2/expr.c    | 25 ++++++++++++++-----------
 gdb/elfread.c        |  7 ++++---
 gdb/findvar.c        |  6 +++---
 gdb/linux-nat-trad.c |  8 ++++----
 gdb/mem-break.c      |  7 +++----
 gdb/printcmd.c       | 44 +++++++++++++++++++++-----------------------
 gdb/remote.c         | 40 ++++++++++++++++++----------------------
 gdb/symfile.c        | 11 +++++------
 gdb/valprint.c       | 17 ++++++++---------
 13 files changed, 103 insertions(+), 111 deletions(-)


base-commit: 85c7cb3c4b70cc484ecf3d72a116503876a28f0a

Comments

Simon Marchi Feb. 28, 2023, 2:47 a.m. UTC | #1 
On 2/27/23 16:29, Andrew Burgess via Gdb-patches wrote:
> Continuing effort to replace alloca calls with C++ data structures,
> here's a bunch of places where gdb::byte_vector can be used.
> 
> This isn't all of them, there's still plenty to work through, but I
> thought I'd see how this lot is received.

Every time I spot an alloca in the wild, I'm also tempted to switch it
to vector or something like that.  But then I think, it's an extra
dynamic allocation for no real gain, so it would just make things worse.

When we know the maximum size a buffer can have, instead of using
alloca, it would better to statically allocate that size on the stack,
and then user an array_view to define the effectively used portion of
it.

For the cases where we don't (I'm thinking of buffer the size of
registers, often small, but there isn't a statically known maximum
length), perhaps an std::vector-like structure with small size
optimization (like std::string has) would be nice.  So you could define
a buffer that would use stack memory up to let's say 128 bytes, and heap
memory above that.  Some references:

  https://stoyannk.wordpress.com/2017/11/18/small-vector-optimization/
  https://llvm.org/doxygen/classllvm_1_1SmallVector.html
  https://github.com/facebook/folly/blob/main/folly/docs/small_vector.md
  https://www.boost.org/doc/libs/1_60_0/doc/html/boost/container/small_vector.html

With something like this, I think we could change pretty much all
allocas without feeling bad about it.

If the license allows, I'd be all for just importing an existing
implementation in our code base, to avoid reinventing the wheel.

Simon
Tom Tromey March 7, 2023, 9:57 p.m. UTC | #2 
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Every time I spot an alloca in the wild, I'm also tempted to switch it
Simon> to vector or something like that.  But then I think, it's an extra
Simon> dynamic allocation for no real gain, so it would just make things worse.

I'd imagine these allocations probably don't matter in most cases, but
on the other hand, alloca is a hazard, as it's easy to accidentally blow
up the stack.

Simon> When we know the maximum size a buffer can have, instead of using
Simon> alloca, it would better to statically allocate that size on the stack,
Simon> and then user an array_view to define the effectively used portion of
Simon> it.

True, and for registers in particular we could just have a #define
somewhere probably.  At least, assuming they have some maximum size
across all supported arches.

Simon> If the license allows, I'd be all for just importing an existing
Simon> implementation in our code base, to avoid reinventing the wheel.

This would also be fine with me but I tend to think in most cases it's
probably unnecessary.  Like, I wouldn't blink if new code came in using
a local vector<> but I might mention a new alloca in a review.

Tom