Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Segmentation fault in ExecBatchBuilder #39583

Closed
zanmato1984 opened this issue Jan 12, 2024 · 2 comments · Fixed by #39585
Closed

[C++] Segmentation fault in ExecBatchBuilder #39583

zanmato1984 opened this issue Jan 12, 2024 · 2 comments · Fixed by #39585
Assignees
Labels
Component: C++ Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. Type: bug
Milestone

Comments

@zanmato1984
Copy link
Collaborator

Describe the bug, including details regarding any error messages, version, and platform.

This is a subsequent issue of #32570.

In the last fix #39234, I didn't realize there were similar issue for fixed size types.

I am now able to produce the segmentation fault using fixed size types, listed in the following UT:

TEST(ExecBatchBuilder, AppendBatchDupRowsFixedSize) {
  std::unique_ptr<MemoryPool> owned_pool = MemoryPool::CreateDefault();
  MemoryPool* pool = owned_pool.get();
  {
    // 63-byte data occupying almost one minimal 64-byte aligned memory region.
    ExecBatch batch_fsb = JSONToExecBatch({fixed_size_binary(9)}, R"([
        ["000000000"],
        ["000000000"],
        ["000000000"],
        ["000000000"],
        ["000000000"],
        ["000000000"],
        ["123456789"]])");  // 9-byte tail row, not aligned to a word.
    ASSERT_EQ(batch_fsb[0].array()->buffers[1]->capacity(), 64);
    ExecBatchBuilder builder;
    uint16_t row_ids[2] = {6, 6};
    ASSERT_OK(builder.AppendSelected(pool, batch_fsb, 2, row_ids, /*num_cols=*/1));
    ExecBatch built = builder.Flush();
    ExecBatch batch_fsb_appended =
        JSONToExecBatch({fixed_size_binary(9)}, R"([["123456789"], ["123456789"]])");
    ASSERT_EQ(batch_fsb_appended, built);
    ASSERT_NE(0, pool->bytes_allocated());
  }
  ASSERT_EQ(0, pool->bytes_allocated());
}

Component(s)

C++

@zanmato1984
Copy link
Collaborator Author

I'm fixing this. Take.

@zanmato1984
Copy link
Collaborator Author

zanmato1984 commented Jan 12, 2024

Take

pitrou pushed a commit that referenced this issue Jan 17, 2024
…ecutive tail rows with the same id may exceed buffer boundary (for fixed size types) (#39585)

### Rationale for this change

#39583 is a subsequent issue of #32570 (fixed by #39234). The last issue and fixed only resolved var length types. It turns out fixed size types have the same issue.

### What changes are included in this PR?

Do the same fix of #39234 for fixed size types.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

* Closes: #39583

Authored-by: zanmato1984 <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 16.0.0 milestone Jan 17, 2024
@raulcd raulcd modified the milestones: 16.0.0, 15.0.1 Jan 18, 2024
idailylife pushed a commit to idailylife/arrow that referenced this issue Jan 18, 2024
…g consecutive tail rows with the same id may exceed buffer boundary (for fixed size types) (apache#39585)

### Rationale for this change

apache#39583 is a subsequent issue of apache#32570 (fixed by apache#39234). The last issue and fixed only resolved var length types. It turns out fixed size types have the same issue.

### What changes are included in this PR?

Do the same fix of apache#39234 for fixed size types.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

* Closes: apache#39583

Authored-by: zanmato1984 <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
clayburn pushed a commit to clayburn/arrow that referenced this issue Jan 23, 2024
…g consecutive tail rows with the same id may exceed buffer boundary (for fixed size types) (apache#39585)

### Rationale for this change

apache#39583 is a subsequent issue of apache#32570 (fixed by apache#39234). The last issue and fixed only resolved var length types. It turns out fixed size types have the same issue.

### What changes are included in this PR?

Do the same fix of apache#39234 for fixed size types.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

* Closes: apache#39583

Authored-by: zanmato1984 <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…g consecutive tail rows with the same id may exceed buffer boundary (for fixed size types) (apache#39585)

### Rationale for this change

apache#39583 is a subsequent issue of apache#32570 (fixed by apache#39234). The last issue and fixed only resolved var length types. It turns out fixed size types have the same issue.

### What changes are included in this PR?

Do the same fix of apache#39234 for fixed size types.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

* Closes: apache#39583

Authored-by: zanmato1984 <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
raulcd pushed a commit that referenced this issue Feb 20, 2024
…ecutive tail rows with the same id may exceed buffer boundary (for fixed size types) (#39585)

### Rationale for this change

#39583 is a subsequent issue of #32570 (fixed by #39234). The last issue and fixed only resolved var length types. It turns out fixed size types have the same issue.

### What changes are included in this PR?

Do the same fix of #39234 for fixed size types.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

* Closes: #39583

Authored-by: zanmato1984 <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@amoeba amoeba added the Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. label Feb 28, 2024
zanmato1984 added a commit to zanmato1984/arrow that referenced this issue Feb 28, 2024
…g consecutive tail rows with the same id may exceed buffer boundary (for fixed size types) (apache#39585)

### Rationale for this change

apache#39583 is a subsequent issue of apache#32570 (fixed by apache#39234). The last issue and fixed only resolved var length types. It turns out fixed size types have the same issue.

### What changes are included in this PR?

Do the same fix of apache#39234 for fixed size types.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

* Closes: apache#39583

Authored-by: zanmato1984 <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
thisisnic pushed a commit to thisisnic/arrow that referenced this issue Mar 8, 2024
…g consecutive tail rows with the same id may exceed buffer boundary (for fixed size types) (apache#39585)

### Rationale for this change

apache#39583 is a subsequent issue of apache#32570 (fixed by apache#39234). The last issue and fixed only resolved var length types. It turns out fixed size types have the same issue.

### What changes are included in this PR?

Do the same fix of apache#39234 for fixed size types.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

* Closes: apache#39583

Authored-by: zanmato1984 <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: C++ Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. Type: bug
Projects
None yet
4 participants