This is an archive of the discontinued LLVM Phabricator instance.

Differential D13016

Check for GLIBC before including execinfo.h
ClosedPublic

Authored by vkalintiris on Sep 21 2015, 6:07 AM.

Details

Reviewers
ovyalov
clayborg
Commits
rGe6f54ef5d086: Check for GLIBC before including execinfo.h
rLLDB248256: Check for GLIBC before including execinfo.h
rL248256: Check for GLIBC before including execinfo.h

Diff Detail

Event Timeline

vkalintiris updated this revision to Diff 35241.Sep 21 2015, 6:07 AM
vkalintiris retitled this revision from to Check for GLIBC before including execinfo.h.
vkalintiris updated this object.
vkalintiris added reviewers: clayborg, ovyalov.
vkalintiris added a subscriber: llvm-commits.
Herald added subscribers: srhines, danalbert, tberghammer. · View Herald TranscriptSep 21 2015, 6:07 AM
clayborg accepted this revision.Sep 21 2015, 9:33 AM
clayborg edited edge metadata.
This revision is now accepted and ready to land.Sep 21 2015, 9:33 AM
danalbert added inline comments.Sep 21 2015, 9:56 AM
source/Host/linux/Host.cpp
17

The !defined(__ANDROID__) is redundant. Android is never glibc.

Closed by commit rL248256: Check for GLIBC before including execinfo.h (authored by vkalintiris). · Explain WhySep 22 2015, 2:55 AM
This revision was automatically updated to reflect the committed changes.
brucem added a subscriber: brucem.Sep 22 2015, 3:01 AM

I missed this the other day. I think this change isn't needed because we can (I think) just not include <execinfo.h> now. It was added to support Host::Backtrace using backtrace, but that code has changed and is no longer here.

vkalintiris added a comment.Sep 22 2015, 3:33 AM
In D13016#250518, @brucem wrote:

I missed this the other day. I think this change isn't needed because we can (I think) just not include <execinfo.h> now. It was added to support Host::Backtrace using backtrace, but that code has changed and is no longer here.

I tested it and it compiles fine without execinfo.h (Mint distro). Should I go on and remove the header? I'll keep an eye on the buildbots just in case.

brucem added a comment.Sep 22 2015, 3:35 AM

I think so. A removal sounds so much better than having an #ifdef. :)

Revision Contents

PathSize
source/
Host/
linux/
2 lines

Diff 35241

source/Host/linux/Host.cpp

//===-- source/Host/linux/Host.cpp ------------------------------*- C++ -*-===// //===-- source/Host/linux/Host.cpp ------------------------------*- C++ -*-===//
// //
// The LLVM Compiler Infrastructure // The LLVM Compiler Infrastructure
// //
// This file is distributed under the University of Illinois Open Source // This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details. // License. See LICENSE.TXT for details.
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// C Includes // C Includes
#include <stdio.h> #include <stdio.h>
#include <sys/utsname.h> #include <sys/utsname.h>
#include <sys/types.h> #include <sys/types.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <dirent.h> #include <dirent.h>
#include <fcntl.h> #include <fcntl.h>
#ifndef __ANDROID__ #if defined(__GLIBC__) && !defined(__ANDROID__)
danalbertUnsubmitted
Not Done
ReplyInline Actions

The !defined(__ANDROID__) is redundant. Android is never glibc.

danalbert: The `!defined(__ANDROID__)` is redundant. Android is never glibc.
#include <execinfo.h> #include <execinfo.h>
#endif #endif
// C++ Includes // C++ Includes
// Other libraries and framework includes // Other libraries and framework includes
// Project includes // Project includes
#include "lldb/Core/Error.h" #include "lldb/Core/Error.h"
#include "lldb/Core/Log.h" #include "lldb/Core/Log.h"
▲ Show 20 Lines • Show All 371 Lines • Show Last 20 Lines