View Issue Details

IDProjectCategoryView StatusLast Update
0004856The Dark ModCodingpublic01.07.2018 04:19
ReporterStefanB Assigned Tostgatilov  
PrioritynormalSeveritynormalReproducibilityalways
Status resolvedResolutionfixed 
Product VersionTDM 2.06 
Fixed in VersionTDM 2.07 
Summary0004856: Bad homegrown offsetof macro implementation may trigger undefined behaviour , break compilation
DescriptionTo see why usage of &(0) is bad even when the pointer is not actually used:
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html

Current GCC versions (8.1) throw an error in this case.
Steps To ReproduceCompile with GCC 8.1

See e.g. http://forums.thedarkmod.com/topic/19499-error-building-206-x64-on-linux/
Additional InformationThe correct way is to use offsetof from stddef.h/cstddef, which typically uses
a compiler builtin function.

The attached patch fixes the problematic occurences.
TagsNo tags attached.
Attached Files
0001-Use-offsetof-instead-of-homegrown-macro-triggering-u.patch (2,196 bytes)   
From 94736ba1a46ba90832ce0afdfbc589079e7640b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Br=C3=BCns?= <stefan.bruens@rwth-aachen.de>
Date: Sun, 1 Jul 2018 01:34:35 +0200
Subject: [PATCH] Use offsetof instead of homegrown macro triggering undefined
 behaviour

Usage of &0 triggers undefined behaviour according to C/C++ standards, use
offsetof from cstddef instead, which typically uses a compiler builtin.
---
 game/StimResponse/StimResponseTimer.cpp | 6 +++---
 idlib/math/Simd_SSE2.cpp                | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/game/StimResponse/StimResponseTimer.cpp b/game/StimResponse/StimResponseTimer.cpp
index 652bc83..f95de42 100644
--- a/game/StimResponse/StimResponseTimer.cpp
+++ b/game/StimResponse/StimResponseTimer.cpp
@@ -15,12 +15,12 @@
 #include "precompiled.h"
 #pragma hdrstop
 
-
+#include <cstddef>
 
 #include "StimResponseTimer.h"
 
-static_assert((size_t)&((TimerValue*)NULL)->Time.Millisecond == 4, "TimerValue type has wrong packing");
-static_assert((size_t)&((TimerValue*)NULL)->Val.Millisecond == 4, "TimerValue type has wrong packing");
+static_assert(offsetof(TimerValue, Time.Millisecond) == 4, "TimerValue type has wrong packing");
+static_assert(offsetof(TimerValue, Val.Millisecond) == 4, "TimerValue type has wrong packing");
 
 /********************************************************************/
 /*                 CStimResponseTimer                               */
diff --git a/idlib/math/Simd_SSE2.cpp b/idlib/math/Simd_SSE2.cpp
index ad31885..65a5cde 100644
--- a/idlib/math/Simd_SSE2.cpp
+++ b/idlib/math/Simd_SSE2.cpp
@@ -16,6 +16,8 @@
 #include "precompiled.h"
 #pragma hdrstop
 
+#include <cstddef>
+
 #include "Simd_Generic.h"
 #include "Simd_MMX.h"
 #include "Simd_SSE.h"
@@ -857,7 +859,7 @@ void VPCALL idSIMD_SSE2::MixedSoundToSamples( short *samples, const float *mixBu
 //suitable for any compiler, OS and bitness  (intrinsics)
 //generally used on Windows 64-bit and all Linuxes
 
-#define OFFSETOF(s, m) ((int)(uintptr_t)&(((s*)NULL)->m))
+#define OFFSETOF(s, m) offsetof(s, m)
 #define SHUF(i0, i1, i2, i3) _MM_SHUFFLE(i3, i2, i1, i0)
 
 #define DOT_PRODUCT(xyz, a, b) \
-- 
2.18.0

Activities

stgatilov

stgatilov

01.07.2018 04:19

administrator   ~0010629

Fixed in svn rev 7478 and 7531.
Not that we care about undefined behavior (that's game, you know), but compile errors must be fixed of course.

Issue History

Date Modified Username Field Change
30.06.2018 23:53 StefanB New Issue
30.06.2018 23:53 StefanB File Added: 0001-Use-offsetof-instead-of-homegrown-macro-triggering-u.patch
01.07.2018 04:19 stgatilov Note Added: 0010629
01.07.2018 04:19 stgatilov Assigned To => stgatilov
01.07.2018 04:19 stgatilov Status new => assigned
01.07.2018 04:19 stgatilov Status assigned => resolved
01.07.2018 04:19 stgatilov Fixed in Version => TDM 2.07
01.07.2018 04:19 stgatilov Resolution open => fixed