Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 315939 - [launcher] Crash in formatVmCommandMsg
Summary: [launcher] Crash in formatVmCommandMsg
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: Framework (show other bugs)
Version: 3.4.2   Edit
Hardware: Macintosh Mac OS X - Carbon (unsup.)
: P3 critical (vote)
Target Milestone: 3.6.1   Edit
Assignee: equinox.framework-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 317200 322291 342748
  Show dependency tree
 
Reported: 2010-06-07 04:49 EDT by linyunzhi CLA
Modified: 2011-04-13 13:52 EDT (History)
5 users (show)

See Also:
tjwatson: review+


Attachments
Crash stacks (21.79 KB, text/plain)
2010-06-07 04:50 EDT, linyunzhi CLA
no flags Details
patch (1.17 KB, patch)
2010-06-07 10:21 EDT, Thomas Watson CLA
no flags Details | Diff
eclipse launcher arguments (2.84 KB, text/plain)
2010-06-07 21:57 EDT, linyunzhi CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description linyunzhi CLA 2010-06-07 04:49:30 EDT
Build Identifier: 3.4.2

In formatVmCommandMsg, there is code to format the message. There is a potential problem in "if (list[index][0] == _T_ECLIPSE('-') && *(ch-1) == _T_ECLIPSE(' '))"  .  "*(ch-1)" may point to an address that is outside the message memory range.  The fix is to change the line to "if (list[index][0] == _T_ECLIPSE('-') && ch != message && *(ch-1) == _T_ECLIPSE(' ')) , this avoids the bad access crash. 

I am trying to create a patch but the network is very slow here today. Since this is a one line fix, so I choose to directly describe here. 


	message = malloc( (length + 5) * sizeof(_TCHAR) );

	/* Format the message such that options (args starting with '-') begin
	   on a new line. Otherwise, the Motif MessageBox does not automatically wrap
	   the messages and the message window can extend beyond both sides of the display. */
	ch = message;
	if(args != NULL) list = args;
	else             list = vmArgs;
	while(list != NULL) {
		for (index = 0; list[index] != NULL; index++)
		{
			if (list[index][0] == _T_ECLIPSE('-') && *(ch-1) == _T_ECLIPSE(' '))
				*(ch-1) = _T_ECLIPSE('\n');
			_tcscpy( ch, list[index] );
			ch += _tcslen( list[index] );
			*ch++ = _T_ECLIPSE(' ');
		}
		if(list == vmArgs) list = progArgs;
		else 			   list = NULL;
	}
	*ch = _T_ECLIPSE('\0');

Reproducible: Always
Comment 1 linyunzhi CLA 2010-06-07 04:50:25 EDT
Created attachment 171237 [details]
Crash stacks
Comment 2 Thomas Watson CLA 2010-06-07 10:21:32 EDT
Created attachment 171265 [details]
patch

I think you mean a patch like this.  The comment 0 seems to just be a paste of the original code unchanged.

This bug looks to be in the 3.6 codebase as well.  This patch is against 3.6.  Could you provide the arguments (eclipse.ini) that you pass the launcher to cause this error?
Comment 3 Andrew Niefer CLA 2010-06-07 10:50:40 EDT
In general the progArgs list would be expected to start with a "-" argument, so I would guess we are always at the least reading the invalid memory location at (ch-1).  I guess it depends on the OS whether a read in a bad location would cause a crash, but if we happen to read a ' ', then the write on the next line would be worse.
Comment 4 linyunzhi CLA 2010-06-07 21:57:20 EDT
Created attachment 171363 [details]
eclipse launcher arguments
Comment 5 linyunzhi CLA 2010-06-07 21:58:34 EDT
The arguments are passed by native rcplauncher to eclipse. You can see the arguments list in the attachments.  The crash is intermittent . 

(In reply to comment #2)
> Created an attachment (id=171265) [details]
> patch
> 
> I think you mean a patch like this.  The comment 0 seems to just be a paste of
> the original code unchanged.
> 
> This bug looks to be in the 3.6 codebase as well.  This patch is against 3.6. 
> Could you provide the arguments (eclipse.ini) that you pass the launcher to
> cause this error?
Comment 6 Thomas Watson CLA 2010-08-10 14:17:07 EDT
I reviewed the patch for 3.6.1.
Comment 7 Andrew Niefer CLA 2010-08-10 15:47:19 EDT
patch released to branch