Skip to content

Fix build and proof failures for CBMC TCP proofs#710

Merged
tony-josi-aws merged 13 commits intoFreeRTOS:dev/IPv6_integrationfrom
tony-josi-aws:fix_cbmc_proof_tcp
Feb 21, 2023
Merged

Fix build and proof failures for CBMC TCP proofs#710
tony-josi-aws merged 13 commits intoFreeRTOS:dev/IPv6_integrationfrom
tony-josi-aws:fix_cbmc_proof_tcp

Conversation

@tony-josi-aws
Copy link
Copy Markdown
Member

This PR fixes the build issues and proof failures for the TCP CBMC proofs post the IPv6 changes.

Test Steps

cd test/cbmc/proofs/
python run-cbmc-proofs.py

Related Issue

Failing CBMC proofs

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

karkhaz and others added 6 commits February 9, 2023 09:57
Prior to this commit, CBMC would emit logging information in plain text
format, which does not contain information required for the CBMC VSCode
debugger. This commit makes CBMC use XML instead of plain text.

Co-authored-by: Mark Tuttle <tuttle@acm.org>
@tony-josi-aws tony-josi-aws requested a review from a team as a code owner February 13, 2023 12:48
Comment thread source/FreeRTOS_TCP_Reception.c Outdated
* more options left.
*/
static void prvReadSackOption( const uint8_t * const pucPtr,
_static void prvReadSackOption( const uint8_t * const pucPtr,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stay away from this modification. There is a CBMC flag (--export-file-local-symbols) which exposes the internal static functions which should be used instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

pxSocket->u.xTCP.txStream = malloc( sizeof( StreamBuffer_t ) );
__CPROVER_assume( pxSocket->u.xTCP.txStream != NULL );
pxSocket->u.xTCP.pxPeerSocket = malloc( sizeof( FreeRTOS_Socket_t ) );
__CPROVER_assume( pxSocket->u.xTCP.pxPeerSocket != NULL );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this assumption being made here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Copy Markdown
Member

@AniruddhaKanhere AniruddhaKanhere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with reservation about comments.

NetworkBufferDescriptor_t * pxNetworkBuffer = ensure_FreeRTOS_NetworkBuffer_is_allocated();
size_t socketSize = sizeof( FreeRTOS_Socket_t );
size_t bufferSize = sizeof( TCPPacket_t );
/* Allocates min. buffer size required for the proof */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment just tells me what is written in the code right below.

Ideally, comments should answer questions like why does the proof need exactly sizeof( TCPPacket_t ) + uxIPHeaderSizeSocket( pxSocket ) and not just sizeof( TCPPacket_t ).

We could improve on these comments.

@tony-josi-aws tony-josi-aws merged commit 907ae56 into FreeRTOS:dev/IPv6_integration Feb 21, 2023
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.

3 participants