Add bounds checking for the "ArrayBounds" saniziter kind.

Use strings for the attribute argument.

Use "Expected" for the SourceRange imports.

Improve error message to use the unknown field's name.

Change sanitizer scope because instructions may be created.

Fix the way the source ranges are imported via the ASTImporter.
Do other minor fixes according to feedback.

Use the new memory location.

This is a precursor to specifying the field with the designated initializer syntax. Therefore, it may be a bit cumbersome.

Fix ICE when the FD isn't found.

Add the ability to specify a "path" to the element count if it resides within a sub-structure.

I think this change is causing build failures. Could you take a look?

Add comment.

Fix failing testcase.

In D148381#4274857, @nickdesaulniers wrote:

Cool!

How about some codegen tests?

I'll add those to the next iteration. :-)

Reformat.

Friendly ping.

Note: This is a work-in-progress, so there's no need for a formal review, though any suggestions will definitely be appreciated. :-)

Fix compilation issue.

In D147580#4250232, @shafik wrote:

I guess I should have waited till I saw: https://reviews.llvm.org/D147673

Remove extraneous space.

Rebase.

Slight typo

Revise with Kees's code.

In D143521#4162405, @nickdesaulniers wrote:

In D143521#4135797, @void wrote:

These look fine, so accepted. One comment: Would it be good to somehow identify in the message the intrinsic each message is folding?

I think that's what II is in the message, right?

Report when there's a non-constant access:

In D144136#4137143, @kees wrote:

Here's a test-case. I'd expect 6 remarks from building this:

/* Build with -Wall -O2 -fstrict-flex-arrays=3 -fsanitize=bounds -Rarray-bounds */
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include <malloc.h>

#define report_size(p, index)      do {    \
    const size_t bdos = __builtin_dynamic_object_size(p, 1); \
    \
    if (__builtin_constant_p(bdos)) { \
        if (bdos == SIZE_MAX) { \
            printf(#p " has unknowable size\n"); \
        } else { \
            printf(#p " has a fixed size: %zu\n", bdos); \
        } \
    } else { \
        printf(#p " has a dynamic size: %zu\n", bdos); \
    } \
    printf(#p "[" #index "] assignment: %d\n", (p)[index] = 15); \
} while (0)

struct fixed {
    unsigned long flags;
    size_t foo;
    int array[16];
};

/* should emit "fixed" */
void do_fixed(struct fixed *p, int index)
{
    report_size(p->array, 0);
    report_size(p->array, index);
}

struct flex {
    unsigned long flags;
    size_t foo;
    int array[];
};

/* should emit "dynamic" */
void do_dynamic(unsigned char count, int index)
{
    /* malloc() is marked with __attribute__((alloc_size(1))) */
    struct flex *p = malloc(sizeof(*p) + count * sizeof(*p->array));
    report_size(p->array, 0);
    report_size(p->array, index);
    free(p);
}

/* should emit "unknowable" */
void do_unknown(struct flex *p, int index)
{
    report_size(p->array, 0);
    report_size(p->array, index);
}

Currently, it only emits once for the compile-time known index with a compile-time known array size:

array.c:31:17: remark: accessing fixed sized array 'int[16]' by 0 [-Rarray-bounds]                  
    report_size(p->array, 0);                                                                                       ^

These look fine, so accepted. One comment: Would it be good to somehow identify in the message the intrinsic each message is folding?

In D144136#4131825, @aaron.ballman wrote:

I'd like to understand what the overhead is for this. How much overhead does this add when the remark is disabled? How much overhead does this add when the remark is enabled?

This will report a ton of information. It's basically only good for piping to a file and using Perl to gather any useful information.

This is why I'm worried about overhead -- there can be *a lot* of array accesses in a complex TU and spitting data out to (potentially) stdout/stderr is *slow*.

In D143300#4108500, @haowei wrote:

FYI, this patch looks like was pushed into a new "Main" branch instead of the actual "main" branch.:

commit f85a9a6452e8f49f9768d66a86434a88a5891614 (origin/Main)
Author: Bill Wendling <morbo@google.com>
Date:   Mon Feb 6 14:26:16 2023 -0800

    [randstruct] Don't allow implicit forward decl to stop struct randomization

It causes git checkout failures on Windows, see error message:

warning: encountered old-style '/ssl/certs/ca-bundle.crt' that should be '%(prefix)//ssl/certs/ca-bundle.crt'
From https://llvm.googlesource.com/a/llvm-project
 * [new branch]                Main       -> Main
error: cannot lock ref 'refs/heads/main': is at f85a9a6452e8f49f9768d66a86434a88a5891614 but expected abbd256a810a0b0c92dda88a3050fc85cb604a9c
 ! abbd256a810a..14ca2e68ff4c  main       -> main  (unable to update local ref)

Windows use case aware but insensitive filesystem and 2 "main" branches will not work out.
See explanation at https://stackoverflow.com/questions/10068640/git-error-on-git-pull-unable-to-update-local-ref/66832220#66832220
I will attempt deleting the "Main" branch to correct this error.

Remove extra line.

LGTM

LGTM

Use TagDecl.

Fix test and add check for EnumDecl.

In D143300#4107132, @nickdesaulniers wrote:

In D143300#4104469, @MaskRay wrote:

clang/test/CodeGen/init-randomized-struct-fwd-decl.c passes without this patch. Is it correct?

Perhaps related to my comment about RecordDecl vs EnumDecl...

or the tests might require a specific randomization seed?

Move test and improve checks.

More tests are always good. :-)

@rsmith Ping. This is blocking some patches I'd like to submit afterwards. :-)

One small comment about the documentation but L:GTM.

Rebase.

Friendly ping!

Still LGTM :-)

LGTM. One suggestion that you can ignore if you wish.

A couple nits, but overall okay.

Could you investigate the build failures?

Sonar ping. :-)

Use EllipsisLoc instead of one of the Brackets.