Hi Evan,
And thanks for getting back to me. I found another problem related to
memory corruption and I've been digging around. I have patches for that
too, but I don't know if they suffice. They sort of break existing
conventions of everything in libgit2 being foreigners and c-pointers.
I'm attaching my patches here in case you have time to look into this.
The attachments here include the fixes in the previous mail too, so please
ignore that one.
>From what I gather, there is no way to allocate memory and return a pointer
to it, safely, in CHICKEN.
Won't the garbage collector potentially overwrite whatever region was
allocated since it has no way of knowing it?
That's why I made (make-oid) and friends return chicken blobs instead. Let
me know if there's a better way!
K.
On Sun, Jun 13, 2021 at 11:19 PM Evan Hanson wrote:
> Hi Kristian,
>
> On 2021-06-12 15:36, Kristian Lein-Mathisen wrote:
> > I think I may have come across a bug in the git egg.
>
> You're right! THanks for pointing this out, I guess you must be the
> first person to use that `frombuffer` procedure.
>
> Everything you've said looks correct to me, I'll try to publish a fix
> for this soon.
>
> Evan
>
From 602f415a5fe769b786efb2abd03a480661b6e0c4 Mon Sep 17 00:00:00 2001
From: Kristian Lein-Mathisen
Date: Sun, 13 Jun 2021 22:56:18 +0200
Subject: [PATCH 1/3] Fix mismatched blob-create-frombuffer signature
Before this patch, the new test case would produce an error like this:
Error: bad argument count - received 2 but expected 3: #
---
git.scm | 2 +-
libgit2.scm | 2 +-
tests/run.scm | 1 +
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/git.scm b/git.scm
index b33cd45..c8762d1 100644
--- a/git.scm
+++ b/git.scm
@@ -785,7 +785,7 @@
(git-blob-lookup
repo*
(cond ((chicken-blob? source)
- (git-blob-create-frombuffer repo* source))
+ (git-blob-create-frombuffer repo* source (number-of-bytes source)))
((string? source)
(if (regular-file? source)
(git-blob-create-fromdisk repo* source)
diff --git a/libgit2.scm b/libgit2.scm
index a1e095c..83334c3 100644
--- a/libgit2.scm
+++ b/libgit2.scm
@@ -295,7 +295,7 @@
(define blob-lookup-prefix (foreign-lambda/allocate blob* git_blob_lookup_prefix repository oid unsigned-int))
(define blob-create-fromdisk(foreign-lambda/allocate oid git_blob_create_fromdisk repository nonnull-c-string))
(define blob-create-fromworkdir (foreign-lambda/allocate oid git_blob_create_fromworkdir repository nonnull-c-string))
-(define blob-create-frombuffer (foreign-lambda/allocate oid git_blob_create_frombuffer repository nonnull-c-string unsigned-int))
+(define blob-create-frombuffer (foreign-lambda/allocate oid git_blob_create_frombuffer repository nonnull-scheme-pointer size_t))
(define blob-id (foreign-lambda/copy oid git_blob_id blob*))
(define blob-free (foreign-lambda void git_blob_free blob*))
(define blob-rawcontent (foreign-lambda c-pointer git_blob_rawcontent blob*))
diff --git a/tests/run.scm b/tests/run.scm
index b73e97f..7ede8da 100644
--- a/tests/run.scm
+++ b/tests/run.scm
@@ -393,6 +393,7 @@
(test-group "blob"
(test-error "blob with nonexistent sha" (blob repo (make-string 40 #\a)))
+ (test "create-blob from memory source" 4 (blob-length (create-blob repo #${41424344})))
(let ((t (commit-tree commit)))
(with-current-directory repo-path
(lambda ()
--
2.31.1
From 621f8a0178d2f66470ca978716b4a62f33f2dd90 Mon Sep 17 00:00:00 2001
From: Kristian Lein-Mathisen
Date: Mon, 14 Jun 2021 01:17:20 +0200
Subject: [PATCH 3/3] Fix for memory corruption
There seems to be a problem with foreign-primitive related
allocations. If the garbage collector runs right after (make-oid) and
friends, the returned pointer is no longer valid.
~/p/chicken-git (master)> cat stress.scm
(import git)
(define repo (create-repository "tmp" #t))
(let loop ()
(create-blob repo #${4142})
(loop))
~/p/chicken-git (master)> valgrind csi -s stress.scm
...
==56676== Invalid read of size 1
==56676==at 0x5479264: git_oid_is_zero (in /usr/lib/libgit2.so.1.1.0)
==56676==by 0x5474509: git_odb_read (in /usr/lib/libgit2.so.1.1.0)
==56676==by 0x547154A: git_object_lookup_prefix (in /usr/lib/libgit2.so.1.1.0)
==56676==by 0x53DF95E: f_12197 (in /usr/lib/chicken/11/git.libgit2.so)
==56676==by 0x48F4FF0: ??? (in /usr/lib/libchicken.so.11)
==56676==by 0x491900A: ??? (in /usr/lib/libchicken.so.11)
==56676==by 0x49267B4: ??? (in /usr/lib/libchicken.so.11)
==56676==by 0x4B012B7: CHICKEN_run (in /usr/lib/libchicken.so.11)
==56676==by 0x4B02813: CHICKEN_main (in /usr/lib/libchicken.so.11)
==56676==by 0x4D7BB24: (below main) (in /usr/lib/libc-2.33.so)
==56676== Address 0x1ffef00514 is on thread 1's stack
==56676== 1046996 bytes below stack pointer
