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

Fix Undefined Behavior #2

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

elvircrn
Copy link

@elvircrn elvircrn commented Oct 14, 2022

Hi all.

I am in the process of reproducing the paper that you guys published.

After following the steps from the README, and testing out the code on the following matrix: https://sparse.tamu.edu/SNAP/CollegeMsg, I received a SEGFAULT.

In particular, here are the steps that I took:

https://github.com/SuperScientificSoftwareLaboratory/TileSpGEMM
cd TileSpGEMM
mkdir data
cd src
make
./test -d 0 -aat 0 [REDACTED]/CollegeMsg/CollegeMsg.mtx

I've done this on the following GPUs with differing CPUs:

RTX 3090
RTX 2080
GTX 1650 Ti

and have therefore managed to produce crashes on multiple machines.

After attempting to integrate this library and taking a closer look at the SEGFAULT, I found three problems in the code with the help of Valgrind:

The first one is a invalid read in line 180 in csr2tile.h. Since ri is compared against rowlen-1, and the for loop goes to BLOCK_SIZE, in the last block, an invalid memory read is done causing a random crash somwhere downstream. This fix changes the logic of the code a bit and I would definitely like to receive feedback here.

The second and third bug seem to be a typo (file: csr2tile.h) and innocent enough at first glance. The memsets touched by this PR write over tile_csr_Col (it should actually zero out tile_csr_Value) and it writes past what was allocated at tile_csr_Col since sizeof(unsigned char) < sizeof(float), causing random crashes depending on how the code is run.

After applying this patch, the code stops crashing.

Please let me know if more information is needed.

Thanks,
Elvir

@elvircrn elvircrn changed the title Fix undefined behavior Fix Undefined Behavior Oct 14, 2022
@elvircrn
Copy link
Author

I've also added one more change to this PR since I've also encountered a crash when CHECK_RESULT=1 flag is added to the Makefile. When the flag is set, the A` matrix is used after being freed. This unblocked me from actually running the tests.

@elvircrn elvircrn mentioned this pull request Oct 14, 2022
@xcx6
Copy link

xcx6 commented May 22, 2024

Could you tell me if you have encountered a situation where matrix information is displayed incorrectly right from the start of reading, and furthermore, when reading the same matrix, the displayed rows and columns are not fixed? Below is my configuration.Note: I ran this on a cloud server, and GPU was not enabled for this run. However, even with GPU enabled, the results are the same.3Q~
EU0)UPF7IH39K)H4L)A%~)Y
U@TWOWX2BXE)~{HKAAUS5(M

@elvircrn
Copy link
Author

Hi xcx6,

It seems like you didn't fully reproduce this error.

Please try using this matrix: https://sparse.tamu.edu/SNAP/CollegeMsg, as reported by my issue.

Thanks,
Elvir

@xcx6
Copy link

xcx6 commented May 22, 2024

Thank you for your response. In fact, I have already tried the three datasets mentioned in your issue as well as the author's dataset, and the problems are all similar.If the dataset does not undergo any additional processing, then the result is like this.

8O}Y%XJ7}7ZX$X9DY`2}N

@xcx6
Copy link

xcx6 commented May 22, 2024

If choose to transpose the matrix, it will result in a segmentation fault error.
)GS9Y@B RK0Y3FPD837E @U

1 similar comment
@xcx6
Copy link

xcx6 commented May 22, 2024

If choose to transpose the matrix, it will result in a segmentation fault error.
)GS9Y@B RK0Y3FPD837E @U

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants