Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix NULL pointer deref when built with gcc -O1 or -O2
The C++ spec says "this" is always NON-NULL, recent versions of gcc will warn
about this and optimizes the "if (this)" we use in some places away:
"warning: nonnull argument ‘this’ compared to NULL [-Wnonnull-compare]"

We rely on "if (this)" checks in several places and refactoring this
is non trivial, so this commit adds a workaround using a helper function
for (this == NULL) checks which is marked as __attribute__((optimize("O0")))
when building with gcc.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
  • Loading branch information
jwrdegoede committed Feb 26, 2018
1 parent 3c674b1 commit 77a34f6
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/lisp/lisp.cpp
Expand Up @@ -860,7 +860,7 @@ size_t LList::GetLength()
size_t ret = 0;

#ifdef TYPE_CHECKING
if (this && item_type(this) != (ltype)L_CONS_CELL)
if (item_type(this) != (ltype)L_CONS_CELL)
{
Print();
lbreak(" is not a sequence\n");
Expand Down Expand Up @@ -1268,7 +1268,7 @@ void LObject::Print()
switch (item_type(this))
{
case L_CONS_CELL:
if (!this)
if (ptr_is_null(this))
{
lprint_string("nil");
}
Expand Down Expand Up @@ -3068,7 +3068,7 @@ LObject *LObject::Eval()

LObject *ret = NULL;

if (this)
if (!ptr_is_null(this))
{
switch (item_type(this))
{
Expand Down
23 changes: 22 additions & 1 deletion src/lisp/lisp.h
Expand Up @@ -250,7 +250,28 @@ class Lisp

static inline LObject *&CAR(void *x) { return ((LList *)x)->m_car; }
static inline LObject *&CDR(void *x) { return ((LList *)x)->m_cdr; }
static inline ltype item_type(void *x) { if (x) return *(ltype *)x; return L_CONS_CELL; }

#ifdef __GNUC__
/*
* C++ spec says "this" is always NON-NULL, recent versions of gcc will warn
* about this and optimizes the "if (this)" we use in some places away:
* "warning: nonnull argument ‘this’ compared to NULL [-Wnonnull-compare]"
* We rely on "if (this)" checks in several places and refactoring this is
* non trivial. So we use this little helper marked with
* __attribute__((optimize("O0"))) to workaround this.
*/
static inline bool __attribute__((optimize("O0"))) ptr_is_null(void *ptr)
{
return ptr == NULL;
}
#else
static inline bool ptr_is_null(void *ptr)
{
return ptr == NULL;
}
#endif

static inline ltype item_type(void *x) { if (!ptr_is_null(x)) return *(ltype *)x; return L_CONS_CELL; }

void perm_space();
void tmp_space();
Expand Down

0 comments on commit 77a34f6

Please sign in to comment.