[ https://issues.apache.org/jira/browse/PROTON-1573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16170140#comment-16170140 ]
Jiri Danek commented on PROTON-1573: ------------------------------------ I think this is resolved already. I'll look for the specific commit. > Undefined behavior in some calls to memmove in Proton > ----------------------------------------------------- > > Key: PROTON-1573 > URL: https://issues.apache.org/jira/browse/PROTON-1573 > Project: Qpid Proton > Issue Type: Bug > Components: proton-c > Affects Versions: proton-c-0.18.0 > Reporter: Jiri Danek > Priority: Trivial > > Proton sometimes calls to memmove with arguments memmove(?, null, 0), that > is, the source pointer is null and length is zero. > According to UndefinedBehaviorSanitizer tool (UBSan), this has undefined > behavior. > {noformat} > SUMMARY: AddressSanitizer: undefined-behavior > /home/jdanek/Work/repos/qpid-proton/proton-c/src/core/codec.c:268:35 in > /home/jdanek/Work/repos/qpid-proton/proton-c/src/core/framing.c:97:39: > runtime error: null pointer passed as argument 2, which is declared to never > be null > {noformat} > It can be "fixed" in the following manner, but I think it is probably not > worth worrying about, unless the code can be somehow restructured that {{n = > 0}} is caught sooner. I verified the fix by running UBSan again. Nothing was > reported this time. > {noformat} > diff --git a/proton-c/src/core/buffer.c b/proton-c/src/core/buffer.c > index c3015f49..f47acd6d 100644 > --- a/proton-c/src/core/buffer.c > +++ b/proton-c/src/core/buffer.c > @@ -170,8 +170,12 @@ int pn_buffer_append(pn_buffer_t *buf, const char > *bytes, size_t size) > size_t tail_space = pni_buffer_tail_space(buf); > size_t n = pn_min(tail_space, size); > > + if (n > 0) { > memmove(buf->bytes + tail, bytes, n); > + } > + if (size - n > 0) { > memmove(buf->bytes, bytes + n, size - n); > + } > > buf->size += size; > > diff --git a/proton-c/src/core/framing.c b/proton-c/src/core/framing.c > index 09bf4bba..d2c355f8 100644 > --- a/proton-c/src/core/framing.c > +++ b/proton-c/src/core/framing.c > @@ -94,7 +94,9 @@ size_t pn_write_frame(char *bytes, size_t available, > pn_frame_t frame) > bytes[5] = frame.type; > pn_i_write16(&bytes[6], frame.channel); > > - memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size); > + if (frame.ex_size > 0) { > + memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size); > + } > memmove(bytes + 4*doff, frame.payload, frame.size); > return size; > } else { > {noformat} > After brief Googling, I believe that although this really is an undefined > behavior, it is reasonable to expect that any real-world implementation of > memmove will be a simple noop as long as {{n = 0}}, which it is always the > case. (https://stackoverflow.com/a/5243068/1047788) > Probably best to ignore this. > The tests that uncover this are for example > proton_tests.engine.ConnectionTest.test_capabilities: > ../proton-c/src/core/framing.c:97:5: runtime error: null pointer passed as > argument 2, which is declared to never be null > proton_tests.engine.CreditTest.testPartialDrain: > ../proton-c/src/core/buffer.c:173:3: runtime error: null pointer passed as > argument 2, which is declared to never be null > ../proton-c/src/core/buffer.c:174:3: runtime error: null pointer passed as > argument 2, which is declared to never be null -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org