{"action":"closed","issue":{"url":"https://api.github.com/repos/JACoders/OpenJK/issues/833","repository_url":"https://api.github.com/repos/JACoders/OpenJK","labels_url":"https://api.github.com/repos/JACoders/OpenJK/issues/833/labels{/name}","comments_url":"https://api.github.com/repos/JACoders/OpenJK/issues/833/comments","events_url":"https://api.github.com/repos/JACoders/OpenJK/issues/833/events","html_url":"https://github.com/JACoders/OpenJK/issues/833","id":153585080,"number":833,"title":"Object alignment requirements not being met","user":{"login":"xycaleth","id":1146859,"avatar_url":"https://avatars.githubusercontent.com/u/1146859?v=3","gravatar_id":"","url":"https://api.github.com/users/xycaleth","html_url":"https://github.com/xycaleth","followers_url":"https://api.github.com/users/xycaleth/followers","following_url":"https://api.github.com/users/xycaleth/following{/other_user}","gists_url":"https://api.github.com/users/xycaleth/gists{/gist_id}","starred_url":"https://api.github.com/users/xycaleth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/xycaleth/subscriptions","organizations_url":"https://api.github.com/users/xycaleth/orgs","repos_url":"https://api.github.com/users/xycaleth/repos","events_url":"https://api.github.com/users/xycaleth/events{/privacy}","received_events_url":"https://api.github.com/users/xycaleth/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/JACoders/OpenJK/labels/T%20-%20Bug","name":"T - Bug","color":"e11d21"}],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2016-05-07T09:35:48Z","updated_at":"2016-05-07T10:19:14Z","closed_at":"2016-05-07T10:19:14Z","body":"# What's the problem?\r\nIn C++, all types have an *[alignment requirement](http://en.cppreference.com/w/cpp/language/object#Alignment)* which states that an object must be aligned to a byte boundary when ever it is instantiated.\r\n\r\nThere are some cases in the code where this alignment requirement is not met. On x86, this is usually not a problem as unaligned memory accesses are automatically dealt with by the CPU. However, in some cases, the compiler can take advantage of the memory alignment and generate instructions which makes use of these assumptions, for example, generating SSE instructions for memory aligned to 16 bytes. In these cases, if the object does not adhere to the requirement, then some of its fields may not meet its alignment requirements - for SSE instructions, memory not aligned to 16 bytes will cause a general protection fault (similar to a segmentation fault).\r\n\r\n# Where is this a problem in the code?\r\nOne example is the `GetMemory` function in `codemp/botlib/l_memory.cpp`. This is used as generic memory allocation for botlib, but it does not adhere to any alignment requirements as it does not know what type will be stored in the memory. System memory allocators such as malloc get around this problem by returning memory with the \"largest fundamental\" alignment. That is, the largest alignment any object can have.\r\n\r\nA concrete example of where this has gone wrong is on OS X 10.11, with the Apple Clang compiler. The following line in https://github.com/JACoders/OpenJK/blob/master/codemp/botlib/l_precomp.cpp#L3091:\r\n\r\n```c++\r\nsource->defines = NULL;\r\n```\r\nIs seemingly harmless. However, `defines` has a 16-byte alignment, and the alignment requirement for `source_t` is also 16 bytes. With this information, clang is able to generate SSE instructions to set this field to zero:\r\n\r\n```asm\r\n    0x100106035 <+149>: xorps  %xmm0, %xmm0\r\n->  0x100106038 <+152>: movaps %xmm0, 0x810(%r12)\r\n```\r\n\r\n`0x810(%r12)` is equivalent to `%r12 + 0x810`, and is the address of the `defines` field. `%r12` is the base address of `source`. However, `source` is allocated from `GetMemory` and has returned:\r\n\r\n```\r\nr12 = 0x000000010329cc28\r\n```\r\n\r\nThe resulting address is `0x10329D438`, which is not 16-byte aligned.\r\n\r\n# How do you propose to fix it?\r\nThere could be any number of cases where this is an issue. @Didz proposes we fix this specific issue with `GetMemory` by returning an address aligned to `alignof(std::max_aligned_t)` - this will give the largest fundamental alignment.\r\n\r\nAny other issues of the same nature should be fixed on a case-by-case basis."}}