This is an archive of the discontinued LLVM Phabricator instance.

Differential D154490

[libunwind] Add missing alloca.h include
AbandonedPublic

Authored by ldionne on Jul 5 2023, 3:32 AM.

Details

Reviewers
StevenvdSchoot
Group Reviewers
Restricted Project
Summary

At the moment we list a set of specifric platforms for which we use alloca for
_LIBUNWIND_REMEMBER_ALLOC. We assume that on those platforms alloca is already
provided somehow. This assumption turns out to be wrong sometimes. In
particular a baremetal system using picolibc is assumed to provide aloca, but
doesn't when compiled with pedantic/without gnu extentions.

This changes the behaviour by looking for different ways an alloca
implementation could be provided and using it when found. If no alloca can be
found we revert back to using malloc.

Diff Detail

Event Timeline

StevenvdSchoot created this revision.Jul 5 2023, 3:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 5 2023, 3:32 AM
StevenvdSchoot requested review of this revision.Jul 5 2023, 3:32 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 5 2023, 3:32 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
Herald added subscribers: libcxx-commits, llvm-commits. · View Herald Transcript
mstorsjo added a subscriber: mstorsjo.Jul 5 2023, 4:33 AM

alloca.h isn’t a standard header, and not all those platforms included in the ifdef do have a header with that name (even if they have the alloca function/macro provided somehow by some other header).

See earlier discussion in e.g. https://reviews.llvm.org/D149635.

Harbormaster completed remote builds in B243173: Diff 537287.Jul 5 2023, 6:01 AM
StevenvdSchoot updated this revision to Diff 537344.Jul 5 2023, 6:44 AM
StevenvdSchoot edited the summary of this revision. (Show Details)
In D154490#4473310, @mstorsjo wrote:

alloca.h isn’t a standard header, and not all those platforms included in the ifdef do have a header with that name (even if they have the alloca function/macro provided somehow by some other header).

See earlier discussion in e.g. https://reviews.llvm.org/D149635.

Ahh, sorry I missed that PR.
I see there has been no activity on that issue since May 10. Based on the comments there, I created a new version.

Herald added subscribers: abidh, krytarowski. · View Herald TranscriptJul 5 2023, 6:44 AM
Harbormaster completed remote builds in B243209: Diff 537344.Jul 5 2023, 10:38 AM
mstorsjo added inline comments.Jul 6 2023, 3:11 PM
libunwind/src/config.h
31

Please avoid changing the indentation of the otherwise unchanged lines (throughout the whole patch).

StevenvdSchoot updated this revision to Diff 538563.Jul 10 2023, 3:08 AM

Undid formatting changes

Harbormaster completed remote builds in B244086: Diff 538563.Jul 10 2023, 4:35 AM
ldionne commandeered this revision.Sep 11 2023, 2:32 PM
ldionne added a reviewer: StevenvdSchoot.
ldionne added a subscriber: ldionne.

[Github PR transition cleanup]

Abandoning in favour of picking up D149635, where we suggested just using __builtin_alloca. All supported compilers should support it.

ldionne abandoned this revision.Sep 11 2023, 2:32 PM
ldionne marked an inline comment as done.

Revision Contents

PathSize
libunwind/
src/
1 line

Diff 537287

libunwind/src/config.h

Show All 22 Lines
// Platform specific configuration defines. // Platform specific configuration defines.
#ifdef __APPLE__ #ifdef __APPLE__
#if defined(FOR_DYLD) #if defined(FOR_DYLD)
#define _LIBUNWIND_SUPPORT_COMPACT_UNWIND 1 #define _LIBUNWIND_SUPPORT_COMPACT_UNWIND 1
#else #else
#define _LIBUNWIND_SUPPORT_COMPACT_UNWIND 1 #define _LIBUNWIND_SUPPORT_COMPACT_UNWIND 1
#define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
#endif #endif
#elif defined(_WIN32) #elif defined(_WIN32)
mstorsjoUnsubmitted
Done
ReplyInline Actions

Please avoid changing the indentation of the otherwise unchanged lines (throughout the whole patch).

mstorsjo: Please avoid changing the indentation of the otherwise unchanged lines (throughout the whole…
#ifdef __SEH__ #ifdef __SEH__
#define _LIBUNWIND_SUPPORT_SEH_UNWIND 1 #define _LIBUNWIND_SUPPORT_SEH_UNWIND 1
#else #else
#define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
#endif #endif
#elif defined(_LIBUNWIND_IS_BAREMETAL) #elif defined(_LIBUNWIND_IS_BAREMETAL)
#if !defined(_LIBUNWIND_ARM_EHABI) #if !defined(_LIBUNWIND_ARM_EHABI)
#define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
▲ Show 20 Lines • Show All 80 Lines • ▼ Show 20 Lines
#define _LIBUNWIND_BUILD_ZERO_COST_APIS #define _LIBUNWIND_BUILD_ZERO_COST_APIS
#endif #endif
#endif #endif
#ifndef _LIBUNWIND_REMEMBER_HEAP_ALLOC #ifndef _LIBUNWIND_REMEMBER_HEAP_ALLOC
#if defined(_LIBUNWIND_REMEMBER_STACK_ALLOC) || defined(__APPLE__) || \ #if defined(_LIBUNWIND_REMEMBER_STACK_ALLOC) || defined(__APPLE__) || \
defined(__linux__) || defined(__ANDROID__) || defined(__MINGW32__) || \ defined(__linux__) || defined(__ANDROID__) || defined(__MINGW32__) || \
defined(_LIBUNWIND_IS_BAREMETAL) defined(_LIBUNWIND_IS_BAREMETAL)
#include <alloca.h>
#define _LIBUNWIND_REMEMBER_ALLOC(_size) alloca(_size) #define _LIBUNWIND_REMEMBER_ALLOC(_size) alloca(_size)
#define _LIBUNWIND_REMEMBER_FREE(_ptr) \ #define _LIBUNWIND_REMEMBER_FREE(_ptr) \
do { \ do { \
} while (0) } while (0)
#elif defined(_WIN32) #elif defined(_WIN32)
#define _LIBUNWIND_REMEMBER_ALLOC(_size) _malloca(_size) #define _LIBUNWIND_REMEMBER_ALLOC(_size) _malloca(_size)
#define _LIBUNWIND_REMEMBER_FREE(_ptr) _freea(_ptr) #define _LIBUNWIND_REMEMBER_FREE(_ptr) _freea(_ptr)
#define _LIBUNWIND_REMEMBER_CLEANUP_NEEDED #define _LIBUNWIND_REMEMBER_CLEANUP_NEEDED
▲ Show 20 Lines • Show All 110 Lines • Show Last 20 Lines