This is an archive of the discontinued LLVM Phabricator instance.

Differential D90615

[libunwind] Add missing #include <alloca.h>
AbandonedPublic

Authored by miyuki on Nov 2 2020, 7:52 AM.

Details

Reviewers
mstorsjo
danielkiss
Group Reviewers
Restricted Project
Summary

The macro/function alloca() is declared in <alloca.h> and in our
configuration it does not get included transitively from other
headers.

This patch adds #include <alloca.h> when alloca() is used.

Diff Detail

Event Timeline

miyuki created this revision.Nov 2 2020, 7:52 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 2 2020, 7:52 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added subscribers: libcxx-commits, llvm-commits. · View Herald Transcript
miyuki requested review of this revision.Nov 2 2020, 7:52 AM
miyuki edited the summary of this revision. (Show Details)
danielkiss accepted this revision.Nov 2 2020, 7:56 AM

LGTM, Thanks!

Harbormaster completed remote builds in B77261: Diff 302282.Nov 2 2020, 7:59 AM
mstorsjo added a comment.Nov 2 2020, 8:08 AM

Unfortunately, not all these systems have an alloca.h, in particular, mingw doesn't.

miyuki added a comment.Nov 2 2020, 8:28 AM

I guess I'll leave it in our downstream version then.

miyuki abandoned this revision.Nov 2 2020, 8:29 AM
danielkiss mentioned this in D85005: [libunwind] Support DW_CFA_remember/restore_state without heap allocation..Dec 10 2020, 1:48 AM
arichardson added a subscriber: arichardson.Mar 25 2021, 5:37 AM
arichardson added inline comments.
libunwind/src/config.h
128

Couldn't this simply be changed to __builtin_alloca() to avoid the header dependency?

mstorsjo added inline comments.Mar 25 2021, 5:51 AM
libunwind/src/config.h
128

Within suitable ifdef __GNUC__ (possibly with some __has_builtin()), then maybe I guess that could be doable

Revision Contents

PathSize
libunwind/
src/
1 line

Diff 302282

libunwind/src/config.h

Show First 20 Lines • Show All 118 Lines • ▼ Show 20 Lines
#if defined(__powerpc64__) && defined(_ARCH_PWR8) #if defined(__powerpc64__) && defined(_ARCH_PWR8)
#define PPC64_HAS_VMX #define PPC64_HAS_VMX
#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)
arichardsonUnsubmitted
Not Done
ReplyInline Actions

Couldn't this simply be changed to __builtin_alloca() to avoid the header dependency?

arichardson: Couldn't this simply be changed to `__builtin_alloca()` to avoid the header dependency?
mstorsjoUnsubmitted
Not Done
ReplyInline Actions

Within suitable ifdef __GNUC__ (possibly with some __has_builtin()), then maybe I guess that could be doable

mstorsjo: Within suitable `ifdef __GNUC__` (possibly with some `__has_builtin()`), then maybe I guess…
#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
#else #else
▲ Show 20 Lines • Show All 105 Lines • Show Last 20 Lines