Skip to content
This repository has been archived by the owner on Oct 3, 2018. It is now read-only.

Validation: Emit errors on pointer casting. #20

Closed
rylimaki opened this issue Jul 30, 2013 · 6 comments
Closed

Validation: Emit errors on pointer casting. #20

rylimaki opened this issue Jul 30, 2013 · 6 comments

Comments

@rylimaki
Copy link
Contributor

No description provided.

@toaarnio
Copy link

toaarnio commented Aug 5, 2013

Could you add a bit more detail here? Which kind of pointer casts do you think should be treated as errors?

@elhigu
Copy link
Contributor

elhigu commented Aug 5, 2013

This is just from webcl specification. As far as I can remember webcl draft had restriction about pointer casts, however I couldn't find it anymore. Maybe it was removed?

I supposed that casting restriction was added originally because of opencl-1.1 specs says that it is platform dependent:

6.2.5 Pointer Casting
Pointers to old and new types may be cast back and forth to each other. Casting a pointer to a
new type represents an unchecked assertion that the address is correctly aligned. The developer
will also need to know the endianness of the OpenCL device and the endianness of the data to
determine how the scalar and vector data elements are stored in memory

Alignment issues worries me a bit if badly aligned memory access causes some dangerous behaviour in some gpus. This is pure speculation though.

@toaarnio
Copy link

toaarnio commented Aug 6, 2013

Pointer casts were originally removed from the kernel language because we thought they would make bounds checking impossible or infeasible, but that turned out not to be the case, so we removed the restriction. Alignment issues were not really considered, as far as I recall.

I would not be surprised if an unaligned memory access terminated the kernel or even crashed the OpenCL driver, but I don't think that alone justifies the removal of pointer casts. In my experience, there are a number of ways to make any given OpenCL driver crash and burn.

The key question is whether an unaligned access could be used to read/write unauthorized memory regions. It seems plausible that trying to read a 32-bit integer from, say, 0xdeadbeef would actually read it from 0xdeadbeec on some implementations. Similarly, using a long16 *, you could potentially read up to 127 bytes out of bounds, which is quite a lot. Do you agree that the validator should counter that by aligning and padding the per-address-space structs according to the largest pointer type that's used in the kernel?

@elhigu
Copy link
Contributor

elhigu commented Aug 12, 2013

After discussion with Tomi decided to add alignment attribute to each address space struct to prevent hypothetical case where driver implementation tries to access pointer from alignment which is before the start of address space. For global memory host should give correctly aligned buffers.

@fcellier
Copy link

x86 allows you to make unaligned accesses: the exception is catched and then CPU does a second unaligned read. ARM is more demanding and often produces a crash... What is worrying me here is the crash of the program in the driver (OpenCL on CPU/ARM ) because it is an open door to the host.

Note that it is not specific to the struct and it seems the unaligned macro is not in all OpenCL compilers.

@elhigu
Copy link
Contributor

elhigu commented Aug 12, 2013

Yes we are aware that unaligned access might cause crashing target device. However there are also many other potential ways to crash different opencl implementations and preventing those is out of scope of this project. This protection is planned to provide protection against hypothetical case where hardware does not crash on unaligned access but returns data from address that is masked to match alignment.

elhigu added a commit that referenced this issue Aug 15, 2013
…rallel localmemory initializer to work with apple driver (original version did crash the i5 cpu compiler) #20
@elhigu elhigu closed this as completed Aug 15, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants