[GHC] #11502: Scrutinize use of 'long' in rts/

#11502: Scrutinize use of 'long' in rts/ -------------------------------------+------------------------------------- Reporter: thomie | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Runtime | Version: 7.10.3 System | Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- In commit 53055bb5023e8cc145ad8a9cd36ac56cee4695b0 (#5656), several uses of `int` in the rts were replaced by `long`, to avoid potential overflows for some calculations. On Windows though, `int` and `long` have the same size (4 bytes). Carefully go through all uses of `long` in `rts/`, and replace by one of the types from `includes/stg/Types.h`: * `long` -> `StgInt` or `StgInt64` * `unsigned long` -> `StgWord` or `StgWord64` -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11502 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11502: Scrutinize use of 'long' in rts/ -------------------------------------+------------------------------------- Reporter: thomie | Owner: MarcelineVQ Type: task | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by MarcelineVQ): * owner: => MarcelineVQ Comment: Is this an appropriate time to change int as well or is it enough to rely on the c-standard minimum int size to keep int usage safe? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11502#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11502: Scrutinize use of 'long' in rts/ -------------------------------------+------------------------------------- Reporter: thomie | Owner: MarcelineVQ Type: task | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): Using `int` is often fine, and sometimes unavoidable since lots of standard C library functions accept or return `int`. There may well be some cases where `int` is used wrongly, but there will be far more correct uses. I can't think of any legitimate use for `long` in the RTS, though. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11502#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11502: Scrutinize use of 'long' in rts/ -------------------------------------+------------------------------------- Reporter: thomie | Owner: MarcelineVQ Type: task | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by thomie): See also https://matt.sh/howto-c and https://github.com/Keith-S-Thompson /how-to-c-response. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11502#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11502: Scrutinize use of 'long' in rts/
-------------------------------------+-------------------------------------
Reporter: thomie | Owner: MarcelineVQ
Type: task | Status: new
Priority: normal | Milestone:
Component: Runtime System | Version: 7.10.3
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by hvr):
Given we're slowly moving to C99-izing the code (se e.g.
db9de7eb3e91820024f673bfdb6fb8064cfed20d), it may make sense to take this
opportunity to use `

#11502: Scrutinize use of 'long' in rts/ -------------------------------------+------------------------------------- Reporter: thomie | Owner: MarcelineVQ Type: task | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by erikd): * cc: erikd (added) Comment: Replying to [comment:4 hvr]:
Given we're slowly moving to C99-izing the code (se e.g. db9de7eb3e91820024f673bfdb6fb8064cfed20d), it may make sense to take this opportunity to use `
`-typedefs instead, e.g. - `int64_t` instead of `StgInt64` - `uint64_t` instead of `StgWord64` - `intptr_t` instead of `StgInt` - `uintptr_t` instead of `StgWord`
I'm no actually sure that just applying this change blindly across the code base is a good idea. There are almost certainly places where `StgWord` is used as a "machine word sized unsigned value" and has no notion of pointer-ness at all. In that case. `size_t` might be a better choice. Similarly, for `StgInt`, there are probably places where `ssize_t` (the signed version of `size_t`) is a better choice. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11502#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11502: Scrutinize use of 'long' in rts/ -------------------------------------+------------------------------------- Reporter: thomie | Owner: MarcelineVQ Type: task | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonmar): `StgWord` is a pretty fundamental notion across the whole of the GHC backend and runtime system: it's the unit of allocation on the heap and stack. So I think for the storage manager we should keep `StgWord` as it is. Places where we're using `StgWord` as just a "machine word sized unsigned value" as @erikd says, we should use something more appropriate. I'd be fine with getting rid of the explicitly sized Stg types in favour of the standard ones. When we originally built this, the standard ones weren't standard enough. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11502#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11502: Scrutinize use of 'long' in rts/ -------------------------------------+------------------------------------- Reporter: thomie | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by MarcelineVQ): * owner: MarcelineVQ => -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11502#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC