Skip to content

Commit 0157dea

Browse files
bors[bot]Robyt3
andauthored
Merge #4971
4971: Fix undefined behavior in fixed point number conversion r=def- a=Robyt3 See #4970: ``` /home/runner/work/ddnet/ddnet/src/base/math.h:50:11: runtime error: left shift of negative value -32 #0 0x198d955 in i2fx(int) /home/runner/work/ddnet/ddnet/src/base/math.h:50:11 #1 0x197ebdf in CLayerQuads::NewQuad(int, int, int, int) /home/runner/work/ddnet/ddnet/src/game/editor/layer_quads.cpp:47:22 #2 0x18ef8d3 in CEditor::Init() /home/runner/work/ddnet/ddnet/src/game/editor/editor.cpp:6360:18 #3 0xa8e5be in CClient::Run() /home/runner/work/ddnet/ddnet/src/engine/client/client.cpp:2917:13 #4 0xaf6726 in main /home/runner/work/ddnet/ddnet/src/engine/client/client.cpp:4458:11 #5 0x7fadd9e610b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2) #6 0x43cefd in _start (/home/runner/work/ddnet/ddnet/san/DDNet+0x43cefd) ``` Upstream teeworlds/teeworlds#3143. ## Checklist - [X] Tested the change ingame (on upstream) - [ ] Provided screenshots if it is a visual change - [ ] Tested in combination with possibly related configuration options - [ ] Written a unit test if it works standalone, system.c especially - [ ] Considered possible null pointers and out of bounds array indexing - [X] Changed no physics that affect existing maps - [X] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional) Co-authored-by: Robert Müller <[email protected]>
2 parents f25fbce + c14889e commit 0157dea

File tree

1 file changed

+36
-12
lines changed

1 file changed

+36
-12
lines changed

src/base/math.h

+36-12
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,31 @@ constexpr inline T mix(const T a, const T b, TB amount)
3838
return a + (b - a) * amount;
3939
}
4040

41-
inline float random_float() { return rand() / (float)(RAND_MAX); }
41+
inline float random_float()
42+
{
43+
return rand() / (float)(RAND_MAX);
44+
}
45+
46+
constexpr int fxpscale = 1 << 10;
4247

4348
// float to fixed
44-
constexpr inline int f2fx(float v) { return (int)(v * (float)(1 << 10)); }
45-
constexpr inline float fx2f(int v) { return v * (1.0f / (1 << 10)); }
49+
constexpr inline int f2fx(float v)
50+
{
51+
return (int)(v * fxpscale);
52+
}
53+
constexpr inline float fx2f(int v)
54+
{
55+
return v / (float)fxpscale;
56+
}
4657

4758
// int to fixed
48-
inline int i2fx(int v)
59+
constexpr inline int i2fx(int v)
4960
{
50-
return v << 10;
61+
return v * fxpscale;
5162
}
52-
inline int fx2i(int v)
63+
constexpr inline int fx2i(int v)
5364
{
54-
return v >> 10;
65+
return v / fxpscale;
5566
}
5667

5768
inline int gcd(int a, int b)
@@ -70,19 +81,32 @@ class fxp
7081
int value;
7182

7283
public:
73-
void set(int v) { value = v; }
74-
int get() const { return value; }
84+
void set(int v)
85+
{
86+
value = v;
87+
}
88+
int get() const
89+
{
90+
return value;
91+
}
7592
fxp &operator=(int v)
7693
{
77-
value = v << 10;
94+
value = i2fx(v);
7895
return *this;
7996
}
8097
fxp &operator=(float v)
8198
{
82-
value = (int)(v * (float)(1 << 10));
99+
value = f2fx(v);
83100
return *this;
84101
}
85-
operator float() const { return value / (float)(1 << 10); }
102+
operator int() const
103+
{
104+
return fx2i(value);
105+
}
106+
operator float() const
107+
{
108+
return fx2f(value);
109+
}
86110
};
87111

88112
constexpr float pi = 3.1415926535897932384626433f;

0 commit comments

Comments
 (0)