Why is glibc's __random_r assigning variables it immediately overwrites?
I was looking for the source for glibc's rand() function, which an answer here links to.
Following the link, I'm puzzled about the code for the __random_r() TYPE_0 branch:
int32_t val = state[0];
val = ((state[0] * 1103515245) + 12345) & 0x7fffffff;
state[0] = val;
*result = val;
What is the point of the val variable, getting assigned and then immediately overwritten? The random_data struct that holds state is nothing unusual.
As one would expect, compiling with -O2 on godbolt gives the same code if you just eliminate val. Is there a known reason for this pattern?
UPDATE: This seems it was an aberration in the version linked to from that answer, I've updated the links there to the 2.28 version. It might have been something that was done temporarily to aid in debugging by making the contents of state[0] easier to see in a local watchlist?
c random glibc
add a comment |
I was looking for the source for glibc's rand() function, which an answer here links to.
Following the link, I'm puzzled about the code for the __random_r() TYPE_0 branch:
int32_t val = state[0];
val = ((state[0] * 1103515245) + 12345) & 0x7fffffff;
state[0] = val;
*result = val;
What is the point of the val variable, getting assigned and then immediately overwritten? The random_data struct that holds state is nothing unusual.
As one would expect, compiling with -O2 on godbolt gives the same code if you just eliminate val. Is there a known reason for this pattern?
UPDATE: This seems it was an aberration in the version linked to from that answer, I've updated the links there to the 2.28 version. It might have been something that was done temporarily to aid in debugging by making the contents of state[0] easier to see in a local watchlist?
c random glibc
add a comment |
I was looking for the source for glibc's rand() function, which an answer here links to.
Following the link, I'm puzzled about the code for the __random_r() TYPE_0 branch:
int32_t val = state[0];
val = ((state[0] * 1103515245) + 12345) & 0x7fffffff;
state[0] = val;
*result = val;
What is the point of the val variable, getting assigned and then immediately overwritten? The random_data struct that holds state is nothing unusual.
As one would expect, compiling with -O2 on godbolt gives the same code if you just eliminate val. Is there a known reason for this pattern?
UPDATE: This seems it was an aberration in the version linked to from that answer, I've updated the links there to the 2.28 version. It might have been something that was done temporarily to aid in debugging by making the contents of state[0] easier to see in a local watchlist?
c random glibc
I was looking for the source for glibc's rand() function, which an answer here links to.
Following the link, I'm puzzled about the code for the __random_r() TYPE_0 branch:
int32_t val = state[0];
val = ((state[0] * 1103515245) + 12345) & 0x7fffffff;
state[0] = val;
*result = val;
What is the point of the val variable, getting assigned and then immediately overwritten? The random_data struct that holds state is nothing unusual.
As one would expect, compiling with -O2 on godbolt gives the same code if you just eliminate val. Is there a known reason for this pattern?
UPDATE: This seems it was an aberration in the version linked to from that answer, I've updated the links there to the 2.28 version. It might have been something that was done temporarily to aid in debugging by making the contents of state[0] easier to see in a local watchlist?
c random glibc
c random glibc
edited Nov 11 at 9:41
asked Nov 11 at 9:17
HostileFork
25.3k777133
25.3k777133
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
Wow, that is indeed some unbelievable garbage code.
There is no justification for this.
And not only is initialization of val not needed, the fact is that state[0] is an int32_t, and multiplication by 1103515245 will trigger undefined behaviour in GCC (integer overflow) on any platform with 32-bit ints (= basically every one). And GCC is the compiler most often used to compile Glibc.
As noted by HostileFork, the code in more recent 2.28 reads:
int32_t val = ((state[0] * 1103515245U) + 12345U) & 0x7fffffff;
state[0] = val;
*result = val;
With this, not only is the useless initialization removed, but the U suffix makes the multiplication happen with unsigned integers, avoiding undefined behaviour. The & 0x7fffffff ensures that the resulting value fits into an int32_t and is positive.
I should have noticed it's not the current stable version in the link, if you look at version 2.28 it doesn't have the extra assignment. :-/ But you might want to raise that issue about the overflow with them...
– HostileFork
Nov 11 at 9:31
@HostileFork yes, that 2.28 seems much saner...
– Antti Haapala
Nov 11 at 9:32
@HostileFork theUsuffix does remove the undefined behaviour - it just depends on the implementation defined conversion of unsigned to signed.
– Antti Haapala
Nov 11 at 9:33
1
Although... that& 0x7fffffffshould kinda assuage that fear.
– StoryTeller
Nov 11 at 9:36
3
"Less garbage" is a viable software quality metric :P
– StoryTeller
Nov 11 at 9:40
|
show 4 more comments
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53247340%2fwhy-is-glibcs-random-r-assigning-variables-it-immediately-overwrites%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
Wow, that is indeed some unbelievable garbage code.
There is no justification for this.
And not only is initialization of val not needed, the fact is that state[0] is an int32_t, and multiplication by 1103515245 will trigger undefined behaviour in GCC (integer overflow) on any platform with 32-bit ints (= basically every one). And GCC is the compiler most often used to compile Glibc.
As noted by HostileFork, the code in more recent 2.28 reads:
int32_t val = ((state[0] * 1103515245U) + 12345U) & 0x7fffffff;
state[0] = val;
*result = val;
With this, not only is the useless initialization removed, but the U suffix makes the multiplication happen with unsigned integers, avoiding undefined behaviour. The & 0x7fffffff ensures that the resulting value fits into an int32_t and is positive.
I should have noticed it's not the current stable version in the link, if you look at version 2.28 it doesn't have the extra assignment. :-/ But you might want to raise that issue about the overflow with them...
– HostileFork
Nov 11 at 9:31
@HostileFork yes, that 2.28 seems much saner...
– Antti Haapala
Nov 11 at 9:32
@HostileFork theUsuffix does remove the undefined behaviour - it just depends on the implementation defined conversion of unsigned to signed.
– Antti Haapala
Nov 11 at 9:33
1
Although... that& 0x7fffffffshould kinda assuage that fear.
– StoryTeller
Nov 11 at 9:36
3
"Less garbage" is a viable software quality metric :P
– StoryTeller
Nov 11 at 9:40
|
show 4 more comments
Wow, that is indeed some unbelievable garbage code.
There is no justification for this.
And not only is initialization of val not needed, the fact is that state[0] is an int32_t, and multiplication by 1103515245 will trigger undefined behaviour in GCC (integer overflow) on any platform with 32-bit ints (= basically every one). And GCC is the compiler most often used to compile Glibc.
As noted by HostileFork, the code in more recent 2.28 reads:
int32_t val = ((state[0] * 1103515245U) + 12345U) & 0x7fffffff;
state[0] = val;
*result = val;
With this, not only is the useless initialization removed, but the U suffix makes the multiplication happen with unsigned integers, avoiding undefined behaviour. The & 0x7fffffff ensures that the resulting value fits into an int32_t and is positive.
I should have noticed it's not the current stable version in the link, if you look at version 2.28 it doesn't have the extra assignment. :-/ But you might want to raise that issue about the overflow with them...
– HostileFork
Nov 11 at 9:31
@HostileFork yes, that 2.28 seems much saner...
– Antti Haapala
Nov 11 at 9:32
@HostileFork theUsuffix does remove the undefined behaviour - it just depends on the implementation defined conversion of unsigned to signed.
– Antti Haapala
Nov 11 at 9:33
1
Although... that& 0x7fffffffshould kinda assuage that fear.
– StoryTeller
Nov 11 at 9:36
3
"Less garbage" is a viable software quality metric :P
– StoryTeller
Nov 11 at 9:40
|
show 4 more comments
Wow, that is indeed some unbelievable garbage code.
There is no justification for this.
And not only is initialization of val not needed, the fact is that state[0] is an int32_t, and multiplication by 1103515245 will trigger undefined behaviour in GCC (integer overflow) on any platform with 32-bit ints (= basically every one). And GCC is the compiler most often used to compile Glibc.
As noted by HostileFork, the code in more recent 2.28 reads:
int32_t val = ((state[0] * 1103515245U) + 12345U) & 0x7fffffff;
state[0] = val;
*result = val;
With this, not only is the useless initialization removed, but the U suffix makes the multiplication happen with unsigned integers, avoiding undefined behaviour. The & 0x7fffffff ensures that the resulting value fits into an int32_t and is positive.
Wow, that is indeed some unbelievable garbage code.
There is no justification for this.
And not only is initialization of val not needed, the fact is that state[0] is an int32_t, and multiplication by 1103515245 will trigger undefined behaviour in GCC (integer overflow) on any platform with 32-bit ints (= basically every one). And GCC is the compiler most often used to compile Glibc.
As noted by HostileFork, the code in more recent 2.28 reads:
int32_t val = ((state[0] * 1103515245U) + 12345U) & 0x7fffffff;
state[0] = val;
*result = val;
With this, not only is the useless initialization removed, but the U suffix makes the multiplication happen with unsigned integers, avoiding undefined behaviour. The & 0x7fffffff ensures that the resulting value fits into an int32_t and is positive.
edited Nov 11 at 9:37
answered Nov 11 at 9:26
Antti Haapala
80.7k16153193
80.7k16153193
I should have noticed it's not the current stable version in the link, if you look at version 2.28 it doesn't have the extra assignment. :-/ But you might want to raise that issue about the overflow with them...
– HostileFork
Nov 11 at 9:31
@HostileFork yes, that 2.28 seems much saner...
– Antti Haapala
Nov 11 at 9:32
@HostileFork theUsuffix does remove the undefined behaviour - it just depends on the implementation defined conversion of unsigned to signed.
– Antti Haapala
Nov 11 at 9:33
1
Although... that& 0x7fffffffshould kinda assuage that fear.
– StoryTeller
Nov 11 at 9:36
3
"Less garbage" is a viable software quality metric :P
– StoryTeller
Nov 11 at 9:40
|
show 4 more comments
I should have noticed it's not the current stable version in the link, if you look at version 2.28 it doesn't have the extra assignment. :-/ But you might want to raise that issue about the overflow with them...
– HostileFork
Nov 11 at 9:31
@HostileFork yes, that 2.28 seems much saner...
– Antti Haapala
Nov 11 at 9:32
@HostileFork theUsuffix does remove the undefined behaviour - it just depends on the implementation defined conversion of unsigned to signed.
– Antti Haapala
Nov 11 at 9:33
1
Although... that& 0x7fffffffshould kinda assuage that fear.
– StoryTeller
Nov 11 at 9:36
3
"Less garbage" is a viable software quality metric :P
– StoryTeller
Nov 11 at 9:40
I should have noticed it's not the current stable version in the link, if you look at version 2.28 it doesn't have the extra assignment. :-/ But you might want to raise that issue about the overflow with them...
– HostileFork
Nov 11 at 9:31
I should have noticed it's not the current stable version in the link, if you look at version 2.28 it doesn't have the extra assignment. :-/ But you might want to raise that issue about the overflow with them...
– HostileFork
Nov 11 at 9:31
@HostileFork yes, that 2.28 seems much saner...
– Antti Haapala
Nov 11 at 9:32
@HostileFork yes, that 2.28 seems much saner...
– Antti Haapala
Nov 11 at 9:32
@HostileFork the
U suffix does remove the undefined behaviour - it just depends on the implementation defined conversion of unsigned to signed.– Antti Haapala
Nov 11 at 9:33
@HostileFork the
U suffix does remove the undefined behaviour - it just depends on the implementation defined conversion of unsigned to signed.– Antti Haapala
Nov 11 at 9:33
1
1
Although... that
& 0x7fffffff should kinda assuage that fear.– StoryTeller
Nov 11 at 9:36
Although... that
& 0x7fffffff should kinda assuage that fear.– StoryTeller
Nov 11 at 9:36
3
3
"Less garbage" is a viable software quality metric :P
– StoryTeller
Nov 11 at 9:40
"Less garbage" is a viable software quality metric :P
– StoryTeller
Nov 11 at 9:40
|
show 4 more comments
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53247340%2fwhy-is-glibcs-random-r-assigning-variables-it-immediately-overwrites%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown