<table border="1" cellspacing="0" cellpadding="8">
    <tr>
        <th>Issue</th>
        <td>
            <a href=https://github.com/llvm/llvm-project/issues/54747>54747</a>
        </td>
    </tr>

    <tr>
        <th>Summary</th>
        <td>
            [Attributor] Heap2stack doesn't handle alignment correctly
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            new issue
      </td>
    </tr>

    <tr>
      <th>Assignees</th>
      <td>
      </td>
    </tr>

    <tr>
      <th>Reporter</th>
      <td>
          jyknight
      </td>
    </tr>
</table>

<pre>
    Attributor's heap2stack pass does not preserve alignment of allocations correctly.

E.g., look at the checked-in test-case llvm/test/Transforms/Attributor/heap_to_stack.ll which creates an under-aligned `alloca i8,i64 4, align 1` from `malloc(4)`:
```
define void @test3() {
; IS________NPM-LABEL: define {{[^@]+}}@test3() {
; IS________NPM-NEXT:    [[TMP1:%.*]] = alloca i8, i64 4, align 1
; IS________NPM-NEXT:    tail call void @no_sync_func(i8* noalias nocapture nofree [[TMP1]])
; IS________NPM-NEXT:    ret void
;
  %1 = tail call noalias i8* @malloc(i64 4)
  tail call void @no_sync_func(i8* %1)
  tail call void @free(i8* %1)
  ret void
}
```

Conversation forked from @durin42's alignment attributes proposal https://discourse.llvm.org/t/rfc-attributes-for-allocator-functions-in-llvm-ir/61464 which mentioned:
> Note that for a function with an implied alignment like malloc we can’t determine the alignment, nor can we trust the align attribute because that is semantically an _under_estimate of the alignment returned by the function. Rather, we should be using an _over_estimate of the alignment promised to users instead. This bug is present in the existing heap-to-stack logic in the Attributor, and is not addressed or affected by this proposal. Fortunately, the heap2stack logic in the Attributor is not included in the set of default passes (though it does run for OpenMP code.)

And to which @jdoerfert replied:
> Given that it is run we should take this as a bug and seriously. I suppose we have to give up on h2s if getAllocAlignment is returning a nullptr, right?

That would work, though note that `getAllocAlignment` only deals with alignment-specified-as-a-parameter; the most common allocator calls don't have a alignment parameter, so that would make this optimization pass nearly never apply.

> Can we teach it about the default alignment of known runtime functions? So spec seems to say malloc returns a pointer aligned properly for any built-in type, which seems to be something we could know (in clang).

Right -- we can't correctly use the "align" attribute on the return value to determine the required alignment, but we provide some other data. The problem is, malloc's alignment rules are rather complicated. The current Standard's rule is that malloc returns memory which is required to be sufficiently-aligned for any fundamental object that fits within the requested size (thus: `malloc(1)` only needs 1-byte alignment, while `malloc(16)` might require 16-byte alignment).

We could hardcode that rule, though that's a little ugly.

Of course, on some implementations, malloc always provides the same alignment (e.g. 16-byte), rather than following the size. On some other implementations, it may follow the size only down to some minimum alignment (e.g. 8 bytes). On such implementations, could a program validly depend on those additional implementation-defined semantics? (That is: may Clang break code saying `void*x = malloc(1); assert((x & 0x4) == 0);` by providing only alignment 1 -- even if the libc implementation in use may always provide higher alignment?)

</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJyNV9ty4kYQ_Rr80iUKBBjzwAO7tpNUZS-VdVXy5hqkEZq1NKPMxSz5-pyekQR4NxcXtsGa7jl9Oafbe1OetjvvrdoHb-wkXzuqpehy50XxQp1wjkojHWnjqbPSSfsqSTTqoFupPZkKHxpTCK-MdlQYa2Xhm9N0MrufzHbp58P0MJ3k76kx5oWEJ19LKmpZvMgyU5q8dD4rhJPUNK_tJH_kP-DXkxXaVca2Dh8uMT4ywmdvniPIadPQsVZFTYWVArYkNAVdSptFnLKkye0soSR1ByDqdklLBhSf0xyPqbKm5XNtPDjJ73Bgg8-TRR8Ev0-v-LGUldKSXo2C--WMIS9gBSOarN_1Jot39MuX5_7r4-cP2a-7dw-_wiX15nwUrxVeD_AyWd1Pcvzlnl__2-nHhz-e2Ce-oqd3Tx8-zxl3vkLad-x0dU-TxT1dJoG-y8J_u_dCNVTAyxi3RhFOuniuguaksecdegUuBfdMITofrMS7ykp5CS-C4hT_961W-njfeDS9QbT5ah7jOuMark5AAHCsZx_uZjD-n7HwHf9qxYH90-k3yFHUH7VS-vne6FdpXSQSoetBjr4pl7MyWKWXeWTnmXuipwQ6vrOmM040VHvfuVj7R7xK5QoTrJNTZtbU2AOzC9-2KrKzeYbrsp7GeMfxRzqDnBkbZopJdztfIoWJaXw_TsjyTI_FA300XoLcYDgckqDBER2Vr5mVqu0ahbjOMTTqRVKqER2hCkJPHvLJ3Wyy2XiQxEvbMk9YMUYj7lmNC3CYbbwNzp9PnNNCe1mI4HpIypGTrQBuLt-J4TxHlXgGy1QL4WAxu7qI6xcsC8j-FJ8MAU3pN4HPlpEAgatNaHBIUnBKH6Jr8_qvnlGwVjl49gZGqDsp7bwU5ZSeakDdhwMjjoqL46ySsJffFDziBhbAzJssqXRjDqoYzlwKJbitS_bD6i3KEt74Ti5OVUGnh8jUuYOm9GisDxqomxN7YJ8XE-Ef7houUbpoQgm__Qkn44yA3InQ-DhP0K7gi0fODjUpn8aLDbHp6VMn9YfPmCOlnJ7VIf7c6Zit1IAgxVcY2kpaLlNsq6te_Em9St1XPhafbzjXyosXmQKHWIiYbk4VKqFMcJhf9Au50CElkq1qgamHyw_wSqEj9HSdo2YVHaTfcffuxtLyXbFvYiuQDk3T-VgMqw61nyweL6N6YoTHiOkI0qeMx9TokU1Qiu-u4ZllNPq4lKJxPcOGh5nrZKEq5CQTLhNZJ6xomUyss1yW1oAyhWlbBDISP-oaj3sNofEpZnHZs6MXgHQmYUvQ2zGdpkPLq7-SjMX1QUthgVNLMIJE171ZDrhY73smS1HElhB7ExKlh8a52jhetDlqLiiuOpMSsvdIXwxx7CikbB1XzInToC-pKlzuzijtGU2_H3DzSwYZdUuf0A-q8XE3OXUysjx23egVVHcGuai5xqxbMQuMi3sbdkUjNMR2cxXqb1x_yrJB6TjL48ZESakwJPM84sLvCy0ziU8pBnoVTYgNeS2RVv4ZlL0UWMYOe74RQb6qMgEnw_JFpfCCBSc-3DeyRfOyxTA0r8aNDQ3vVpjmNoof9w94h9aRZXJSBMSCk188uCRsGR2wGXMidsubSrSyNfbUJzfypsffpzhUlSoUXDancZcbSoSyl4KBYeqZ_VfksB89yic6KD3mBDoMS6f-kkl6Ao_Iq21vnra9RCktZelonu1P_s3YAVJEc2V421u2sbh9BDS__c76uhd-H5qmRqJY7hJ6ztaFBvDfUhkwKb3H3eHwhj-fKkozns3QJbG8PGhlTE5ixlhTADqKkxuawSWRRh4vCo2oJDb2IQaOj8UrFR2IWKnh68i9H82R1yl90ped9QMAist_6m1Hw17EmNDMVvaAdlZtaH-A6I4YkONcxvsCt833N6XECg7yAMlitqgySiWmS5moxMKOiajYCB107SVL63k5bgxRW4DiKW0S3D0cy3tmOe3xb8dLHFmsNpwV9ENc-_Ldt7ifXrcZNJjHoPVxtb_DkfyWZt-Wcc1f3LPBLJ3jvsKATsVixzFZ57zMWUwkDzqVFoxG7Ys3sfAkZmlhvNfFpxotO6hg7FGMpmHm3pTbRblZbMSNV-i7Lfb2i80C_0_8fN4JeIIPM0OXzWUvjfJ2E2yzvd5ODyBp2E8hI_jQ_-sX103gYz7jo3IucL0fV8v1cn1Tb9erzWYuNreLslxv1iux2VdiNisrsSjkei7Km0bsZeMYLvRTyyNFF3gPzDdqm8_yfLacrWbzVT6bT4vloprNZF7kc7EWyxXWClRcNeO-fGO3ERIWBIeHDfYvd36IMrIoxeywfxHQWHb79fSiWQ1u4t3biP1vDIQqTg">