Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined left shift of negative value. #7010

Closed
vicuna opened this issue Oct 7, 2015 · 2 comments
Closed

Fix undefined left shift of negative value. #7010

vicuna opened this issue Oct 7, 2015 · 2 comments

Comments

@vicuna
Copy link

vicuna commented Oct 7, 2015

Original bug ID: 7010
Reporter: bryantosaurus
Assigned to: @mshinwell
Status: closed (set by @xavierleroy on 2017-02-16T14:16:43Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.3
Fixed in version: 4.03.0+dev / +beta1
Category: configure and build/install
Monitored by: @diml

Bug description

19:30:23 ~/3rd/ocaml> git format-patch trunk --stdout | cat

From 749045d74ec9617e651901fbfe98d135061ec7e5 Mon Sep 17 00:00:00 2001
From: bryant <bryant@defrag.in>
Date: Wed, 7 Oct 2015 19:18:13 +0200
Subject: [PATCH] fix undefined left shift of negative value.

---
 byterun/caml/mlvalues.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/byterun/caml/mlvalues.h b/byterun/caml/mlvalues.h
index 3b94d01..025bee1 100644
--- a/byterun/caml/mlvalues.h
+++ b/byterun/caml/mlvalues.h
@@ -68,7 +68,7 @@ typedef uintnat mark_t;
 
 /* Conversion macro names are always of the form  "to_from". */
 /* Example: Val_long as in "Val from long" or "Val of long". */
-#define Val_long(x)     (((intnat)(x) << 1) + 1)
+#define Val_long(x)     ((intnat) (((uintnat)(x) << 1)) + 1)
 #define Long_val(x)     ((x) >> 1)
 #define Max_long (((intnat)1 << (8 * sizeof(value) - 2)) - 1)
 #define Min_long (-((intnat)1 << (8 * sizeof(value) - 2)))
-- 
1.7.10.4

Left-shifting signed negative values is undefined behavior. From 6.5.7 of http://open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf :

If E1 has a signed
type and nonnegative value, and E1 × 2 E2 is representable in the result type, then that is
the resulting value; otherwise, the behavior is undefined.

This patch also has the pleasant side effect of permitting build with clang.

Thanks,
Bryant

Steps to reproduce

compile ocaml with clang. ./configure -cc clang.

@vicuna
Copy link
Author

vicuna commented Oct 8, 2015

Comment author: @mshinwell

I just built 4.02.3 using clang on a Mac (clang-700.0.72) and didn't get this error. Which version of clang are you using? Can you paste the command line of the failing compilation command, and the compiler output?

I suspect that the change introduced in trunk (as part of a move to a higher optimization level) to use the "-fwrapv" compiler option will fix this problem.

@vicuna
Copy link
Author

vicuna commented Nov 22, 2015

Comment author: @xavierleroy

Fixed as part of #255 (#255).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants